Switching Interfaces to be instances

They were previously classes, which made a lot of things stupider
than they needed to be.

This will likely involve some porting, but not much.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>

17 files changed, 127 insertions, 103 deletions

diff --git a/lib/puppet/application/interface_base.rb b/lib/puppet/application/interface_base.rb
index 1dd1f76c2..9a6c8d9ec 100644
--- a/lib/puppet/application/interface_base.rb
+++ b/lib/puppet/application/interface_base.rb
@@ -68,8 +68,10 @@ class Puppet::Application::InterfaceBase < Puppet::Application
 
     @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
 
-    @interface = Puppet::Interface.interface(@type).new
-    @format ||= @interface.class.default_format || :pson
+    unless @interface = Puppet::Interface.interface(@type)
+      raise "Could not find interface '#{@type}'"
+    end
+    @format ||= @interface.default_format || :pson
 
     validate
 
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 2e52de43d..901e83af6 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -1,60 +1,31 @@
 require 'puppet'
+require 'puppet/util/autoload'
 
 class Puppet::Interface
+  require 'puppet/interface/action_manager'
 
-  class << self
-    attr_accessor :default_format, :abstract
-
-    # Is this an actual interface, or a base class for others?
-    def abstract?
-      abstract
-    end
-
-    def set_default_format(format)
-      self.default_format = format.to_sym
-    end
-  end
-
+  include Puppet::Interface::ActionManager
+  extend Puppet::Interface::ActionManager
   # This is just so we can search for actions.  We only use its
   # list of directories to search.
   def self.autoloader
-    require 'puppet/util/autoload'
     @autoloader ||= Puppet::Util::Autoload.new(:application, "puppet/interface")
   end
 
-  # Declare that this app can take a specific action, and provide
-  # the code to do so.
-  def self.action(name, &block)
-    @actions ||= []
-    name = name.to_s.downcase.to_sym
-    raise "Action #{name} already defined for #{self}" if actions.include?(name)
-
-    @actions << name
-
-    define_method(name, &block)
-  end
-
-  def self.actions
-    @actions ||= []
-    (if superclass.respond_to?(:actions)
-      @actions + superclass.actions
-    else
-      @actions
-    end).sort { |a,b| a.to_s <=> b.to_s }
-  end
-
   # Return an interface by name, loading from disk if necessary.
   def self.interface(name)
-    require "puppet/interface/#{name.to_s.downcase}"
-    self.const_get(name.to_s.capitalize)
+    @interfaces ||= {}
+    unless @interfaces[unify_name(name)]
+      require "puppet/interface/#{unify_name(name)}"
+    end
+    @interfaces[unify_name(name)]
   rescue Exception => detail
     puts detail.backtrace if Puppet[:trace]
     $stderr.puts "Unable to find interface '#{name.to_s}': #{detail}."
-    Kernel::exit(1)
   end
 
   # Try to find actions defined in other files.
-  def self.load_actions
+  def self.load_actions(name)
     path = "puppet/interface/#{name}"
 
     autoloader.search_directories.each do |dir|
@@ -68,17 +39,43 @@ class Puppet::Interface
     end
   end
 
+  def self.register_interface(name, instance)
+    @interfaces ||= {}
+    @interfaces[unify_name(name)] = instance
+    const_set(name2const(name), instance)
+  end
+
+  def self.unload_interface(name)
+    @interfaces ||= {}
+    @interfaces.delete(unify_name(name))
+    const = name2const(name)
+    const_get(const)
+    remove_const(const)
+  rescue
+    # nothing - if the constant-getting fails, just return
+  end
+
+  def self.unify_name(name)
+    name.to_s.downcase.to_sym
+  end
+
+  def self.name2const(name)
+    name.to_s.capitalize
+  end
+
+  attr_accessor :default_format
+
+  def set_default_format(format)
+    self.default_format = format.to_sym
+  end
+
   # Return the interface name.
-  def self.name
+  def name
     @name || self.to_s.sub(/.+::/, '').downcase
   end
 
   attr_accessor :type, :verb, :name, :arguments
 
-  def action?(name)
-    self.class.actions.include?(name.to_sym)
-  end
-
   # Print the configuration for the current terminus class
   action :showconfig do |*args|
     if t = indirection.terminus_class
@@ -88,12 +85,22 @@ class Puppet::Interface
     end
   end
 
-  def initialize(options = {})
+  def initialize(name, options = {}, &block)
+    @name = name
+
+    @default_format = :pson
     options.each { |opt, val| send(opt.to_s + "=", val) }
 
-    Puppet::Util::Log.newdestination :console
+    # 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)
 
-    self.class.load_actions
-  end
+    Puppet::Interface.load_actions(name)
 
+    if block_given?
+      instance_eval(&block)
+    end
+  end
 end
diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
new file mode 100644
index 000000000..27a982929
--- /dev/null
+++ b/lib/puppet/interface/action_manager.rb
@@ -0,0 +1,32 @@
+module Puppet::Interface::ActionManager
+  # Declare that this app can take a specific action, and provide
+  # 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)
+
+    @actions << name
+    if self.is_a?(Class)
+      define_method(name, &block)
+    else
+      meta_def(name, &block)
+    end
+  end
+
+  def actions
+    @actions ||= []
+    result = @actions.dup
+
+    if self.is_a?(Class) and superclass.respond_to?(:actions)
+      result += superclass.actions
+    elsif self.class.respond_to?(:actions)
+      result += self.class.actions
+    end
+    result.sort { |a,b| a.to_s <=> b.to_s }
+  end
+
+  def action?(name)
+    actions.include?(name.to_sym)
+  end
+end
diff --git a/lib/puppet/interface/catalog.rb b/lib/puppet/interface/catalog.rb
index 85aa2f37a..b2ed08f92 100644
--- a/lib/puppet/interface/catalog.rb
+++ b/lib/puppet/interface/catalog.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Catalog < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:catalog) do
 end
diff --git a/lib/puppet/interface/certificate.rb b/lib/puppet/interface/certificate.rb
index 48ca2c20f..52ba4e3b8 100644
--- a/lib/puppet/interface/certificate.rb
+++ b/lib/puppet/interface/certificate.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Certificate < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:certificate) do
 end
diff --git a/lib/puppet/interface/certificate_request.rb b/lib/puppet/interface/certificate_request.rb
index 29dc73b9a..77b485f8e 100644
--- a/lib/puppet/interface/certificate_request.rb
+++ b/lib/puppet/interface/certificate_request.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Certificate_request < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:certificate_request) do
 end
diff --git a/lib/puppet/interface/certificate_revocation_list.rb b/lib/puppet/interface/certificate_revocation_list.rb
index 144d5ef61..ee1e6a8c4 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'
 
-class Puppet::Interface::Certificate_revocation_list < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:certificate_revocation_list) do
 end
diff --git a/lib/puppet/interface/facts.rb b/lib/puppet/interface/facts.rb
index 42ba1fb81..7b269e0d0 100644
--- a/lib/puppet/interface/facts.rb
+++ b/lib/puppet/interface/facts.rb
@@ -1,6 +1,7 @@
 require 'puppet/interface/indirector'
+require 'puppet/node/facts'
 
-class Puppet::Interface::Facts < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(: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 98a869153..9060c4042 100644
--- a/lib/puppet/interface/file.rb
+++ b/lib/puppet/interface/file.rb
@@ -1,7 +1,5 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::File < Puppet::Interface::Indirector
-  def self.indirection_name
-    :file_bucket_file
-  end
+class Puppet::Interface::Indirector.new(:file) do
+  set_indirection_name :file_bucket_file
 end
diff --git a/lib/puppet/interface/indirector.rb b/lib/puppet/interface/indirector.rb
index 507826b91..feb356d85 100644
--- a/lib/puppet/interface/indirector.rb
+++ b/lib/puppet/interface/indirector.rb
@@ -2,27 +2,14 @@ require 'puppet'
 require 'puppet/interface'
 
 class Puppet::Interface::Indirector < Puppet::Interface
-
-  # This is just a base class.
-  @abstract = true
-
-  # Here's your opportunity to override the indirection name.  By default
-  # it will be the same name as the interface.
-  def self.indirection_name
-    name.to_sym
+  def self.indirections
+    Puppet::Indirector::Indirection.instances.collect { |t| t.to_s }.sort
   end
 
-  # Return an indirection associated with an interface, if one exists
-  # One usually does.
-  def self.indirection
-    unless @indirection
-      Puppet.info("Could not find terminus for #{indirection_name}") unless @indirection = Puppet::Indirector::Indirection.instance(indirection_name)
-    end
-    @indirection
+  def self.terminus_classes(indirection)
+    Puppet::Indirector::Terminus.terminus_classes(indirection.to_sym).collect { |t| t.to_s }.sort
   end
 
-  attr_accessor :from, :indirection
-
   action :destroy do |name, *args|
     call_indirection_method(:destroy, name, *args)
   end
@@ -39,16 +26,25 @@ class Puppet::Interface::Indirector < Puppet::Interface
     call_indirection_method(:search, name, *args)
   end
 
-  def indirection
-    self.class.indirection
-  end
+  attr_accessor :from
 
-  def initialize(options = {})
-    options.each { |opt, val| send(opt.to_s + "=", val) }
+  def indirection_name
+    @indirection_name || name.to_sym
+  end
 
-    Puppet::Util::Log.newdestination :console
+  # Here's your opportunity to override the indirection name.  By default
+  # it will be the same name as the interface.
+  def set_indirection_name(name)
+    @indirection_name = name
+  end
 
-    self.class.load_actions
+  # Return an indirection associated with an interface, if one exists
+  # One usually does.
+  def indirection
+    unless @indirection
+      Puppet.info("Could not find terminus for #{indirection_name}") unless @indirection = Puppet::Indirector::Indirection.instance(indirection_name)
+    end
+    @indirection
   end
 
   def set_terminus(from)
@@ -64,21 +60,9 @@ class Puppet::Interface::Indirector < Puppet::Interface
       result = indirection.send(method, name, *args)
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
-      raise "Could not call #{method} on #{type}: #{detail}"
-    end
-
-    unless result
-      raise "Could not #{method} #{indirection.name} for #{name}"
+      raise "Could not call '#{method}' on '#{indirection_name}': #{detail}"
     end
 
     result
   end
-
-  def indirections
-      Puppet::Indirector::Indirection.instances.collect { |t| t.to_s }.sort
-  end
-
-  def terminus_classes(indirection)
-      Puppet::Indirector::Terminus.terminus_classes(indirection).collect { |t| t.to_s }.sort
-  end
 end
diff --git a/lib/puppet/interface/inventory.rb b/lib/puppet/interface/inventory.rb
index 16b216b8b..9b597c6ae 100644
--- a/lib/puppet/interface/inventory.rb
+++ b/lib/puppet/interface/inventory.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Inventory < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:inventory) do
 end
diff --git a/lib/puppet/interface/key.rb b/lib/puppet/interface/key.rb
index 17b661da1..9343891d0 100644
--- a/lib/puppet/interface/key.rb
+++ b/lib/puppet/interface/key.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Key < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:key) do
 end
diff --git a/lib/puppet/interface/node.rb b/lib/puppet/interface/node.rb
index 5d9efa932..7d7362d8b 100644
--- a/lib/puppet/interface/node.rb
+++ b/lib/puppet/interface/node.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Node < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:node) do
 end
diff --git a/lib/puppet/interface/report.rb b/lib/puppet/interface/report.rb
index fd6f45f16..e7b916527 100644
--- a/lib/puppet/interface/report.rb
+++ b/lib/puppet/interface/report.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Report < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:report) do
 end
diff --git a/lib/puppet/interface/resource.rb b/lib/puppet/interface/resource.rb
index deed0a533..65f2dec7a 100644
--- a/lib/puppet/interface/resource.rb
+++ b/lib/puppet/interface/resource.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Resource < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:resource) do
 end
diff --git a/lib/puppet/interface/resource_type.rb b/lib/puppet/interface/resource_type.rb
index 6892926f0..bf16652a8 100644
--- a/lib/puppet/interface/resource_type.rb
+++ b/lib/puppet/interface/resource_type.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Resource_type < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:resource_type) do
 end
diff --git a/lib/puppet/interface/status.rb b/lib/puppet/interface/status.rb
index 86ccab6e1..1a1d349d1 100644
--- a/lib/puppet/interface/status.rb
+++ b/lib/puppet/interface/status.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-class Puppet::Interface::Status < Puppet::Interface::Indirector
+Puppet::Interface::Indirector.new(:status) do
 end