(#6786) Change interface storage and access.

Ruby's namespace mechanism introduced a number of problems, including
incorrect name resolution for common and simple cases.  Given that,
we've refactored back to class-level data structures with accessor
methods available.

The current method names are unlikely to be the final UI.

Reviewed-By: Daniel Pittman

20 files changed, 72 insertions, 82 deletions

diff --git a/lib/puppet/application/configurer.rb b/lib/puppet/application/configurer.rb
index 70d24814e..378364430 100644
--- a/lib/puppet/application/configurer.rb
+++ b/lib/puppet/application/configurer.rb
@@ -17,7 +17,7 @@ class Puppet::Application::Configurer < Puppet::Application
   end
 
   def run_command
-    report = Puppet::Interface::Configurer.synchronize(Puppet[:certname])
-    Puppet::Interface::Report.submit(report)
+    report = Puppet::Interface.interface(:configurer).synchronize(Puppet[:certname])
+    Puppet::Interface.interface(:report).submit(report)
   end
 end
diff --git a/lib/puppet/application/interface_base.rb b/lib/puppet/application/interface_base.rb
index f2c147f1f..654674df5 100644
--- a/lib/puppet/application/interface_base.rb
+++ b/lib/puppet/application/interface_base.rb
@@ -75,9 +75,10 @@ class Puppet::Application::InterfaceBase < Puppet::Application
 
     @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
 
-    unless @interface = Puppet::Interface.const_get(@type)
+    unless Puppet::Interface.interface?(@type)
       raise "Could not find interface '#{@type}'"
     end
+    @interface = Puppet::Interface.interface(@type)
     @format ||= @interface.default_format
 
     # We copy all of the app options to the interface.
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 38841d948..0ec0f803f 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -9,6 +9,8 @@ class Puppet::Interface
 
   include Puppet::Util
 
+  @interfaces = {}
+
   # This is just so we can search for actions.  We only use its
   # list of directories to search.
   # Can't we utilize an external autoloader, or simply use the $LOAD_PATH? -pvb
@@ -34,31 +36,33 @@ class Puppet::Interface
         end
       end
     end
-    Puppet::Interface.constants.map { |c| c.to_s.downcase }
+    return @interfaces.keys
   end
 
-  def self.const_missing(name)
-    require "puppet/interface/#{name.to_s.downcase}"
-    const_get(name) if const_defined?(name)
+  def self.interface?(name)
+    name = underscorize(name)
+    require "puppet/interface/#{name}" unless @interfaces.has_key? name
+    return @interfaces.has_key? name
   rescue LoadError
-    nil
+    return false
   end
 
-  def self.const_get(name)
-    name = constantize(name)
-    super(name)
+  def self.interface(name, &blk)
+    interface = interface?(name) ? @interfaces[underscorize(name)] : new(name)
+    interface.instance_eval(&blk) if block_given?
+    return interface
   end
 
   def self.register_interface(name, instance)
-    const_set(constantize(name), instance)
+    @interfaces[underscorize(name)] = instance
   end
 
-  def self.constantize(name)
+  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.split(/[-_]/).map { |x| x.capitalize }.join
+    name.to_s.downcase.split(/[-_]/).join('_').to_sym
   end
 
   attr_accessor :default_format
@@ -75,7 +79,7 @@ class Puppet::Interface
   attr_accessor :type, :verb, :name, :arguments, :options
 
   def initialize(name, options = {}, &block)
-    @name = name
+    @name = self.class.underscorize(name)
 
     @default_format = :pson
     options.each { |opt, val| send(opt.to_s + "=", val) }
@@ -118,6 +122,6 @@ class Puppet::Interface
   end
 
   def to_s
-    name.to_s
+    "Puppet::Interface(#{name})"
   end
 end
diff --git a/lib/puppet/interface/catalog.rb b/lib/puppet/interface/catalog.rb
index f99d0881a..34a1d8119 100644
--- a/lib/puppet/interface/catalog.rb
+++ b/lib/puppet/interface/catalog.rb
@@ -25,7 +25,7 @@ Puppet::Interface::Indirector.new(:catalog) do
     facts_to_upload = {:facts_format => :b64_zlib_yaml, :facts => CGI.escape(facts.render(:b64_zlib_yaml))}
     catalog = nil
     retrieval_duration = thinmark do
-      catalog = Puppet::Interface::Catalog.find(certname, facts_to_upload)
+      catalog = Puppet::Interface.interface(:catalog).find(certname, facts_to_upload)
     end
     catalog = catalog.to_ral
     catalog.finalize
diff --git a/lib/puppet/interface/catalog/select.rb b/lib/puppet/interface/catalog/select.rb
index 4bb49315c..082d93c34 100644
--- a/lib/puppet/interface/catalog/select.rb
+++ b/lib/puppet/interface/catalog/select.rb
@@ -1,8 +1,10 @@
 # Select and show a list of resources of a given type.
-Puppet::Interface::Catalog.action :select do |*args|
-  host = args.shift
-  type = args.shift
-  catalog = Puppet::Resource::Catalog.indirection.find(host)
+Puppet::Interface.interface(:catalog) do
+  action :select do |*args|
+    host = args.shift
+    type = args.shift
+    catalog = Puppet::Resource::Catalog.indirection.find(host)
 
-  catalog.resources.reject { |res| res.type != type }.each { |res| puts res }
+    catalog.resources.reject { |res| res.type != type }.each { |res| puts res }
+  end
 end
diff --git a/lib/puppet/interface/configurer.rb b/lib/puppet/interface/configurer.rb
index 42e950fa3..c1a28b2e7 100644
--- a/lib/puppet/interface/configurer.rb
+++ b/lib/puppet/interface/configurer.rb
@@ -2,12 +2,9 @@ require 'puppet/interface'
 
 Puppet::Interface.new(:configurer) do
   action(:synchronize) do |certname|
-    facts = Puppet::Interface::Facts.find(certname)
-
-    catalog = Puppet::Interface::Catalog.download(certname, facts)
-
-    report = Puppet::Interface::Catalog.apply(catalog)
-
+    facts = Puppet::Interface.interface(:facts).find(certname)
+    catalog = Puppet::Interface.interface(:catalog).download(certname, facts)
+    report = Puppet::Interface.interface(:catalog).apply(catalog)
     report
   end
 end
diff --git a/spec/unit/interface/catalog_spec.rb b/spec/unit/interface/catalog_spec.rb
index 574842754..8eb0040ff 100644
--- a/spec/unit/interface/catalog_spec.rb
+++ b/spec/unit/interface/catalog_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/catalog'
 
-describe Puppet::Interface::Catalog do
+describe Puppet::Interface.interface(:catalog) do
   before do
-    @interface = Puppet::Interface::Catalog
+    @interface = Puppet::Interface.interface(:catalog)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/certificate_request_spec.rb b/spec/unit/interface/certificate_request_spec.rb
index fa9e819b4..8a613e5e5 100644
--- a/spec/unit/interface/certificate_request_spec.rb
+++ b/spec/unit/interface/certificate_request_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_request'
 
-describe Puppet::Interface::CertificateRequest do
+describe Puppet::Interface.interface(:certificate_request) do
   before do
-    @interface = Puppet::Interface::CertificateRequest
+    @interface = Puppet::Interface.interface(:certificate_request)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/certificate_revocation_list_spec.rb b/spec/unit/interface/certificate_revocation_list_spec.rb
index 3fc981bd4..8ee341bef 100644
--- a/spec/unit/interface/certificate_revocation_list_spec.rb
+++ b/spec/unit/interface/certificate_revocation_list_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_revocation_list'
 
-describe Puppet::Interface::CertificateRevocationList do
+describe Puppet::Interface.interface(:certificate_revocation_list) do
   before do
-    @interface = Puppet::Interface::CertificateRevocationList
+    @interface = Puppet::Interface.interface(:certificate_revocation_list)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/certificate_spec.rb b/spec/unit/interface/certificate_spec.rb
index 6b5f9224b..47ddcb52e 100644
--- a/spec/unit/interface/certificate_spec.rb
+++ b/spec/unit/interface/certificate_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate'
 
-describe Puppet::Interface::Certificate do
+describe Puppet::Interface.interface(:certificate) do
   before do
-    @interface = Puppet::Interface::Certificate
+    @interface = Puppet::Interface.interface(:certificate)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/config_spec.rb b/spec/unit/interface/config_spec.rb
index 683e8abae..79c65f2ac 100644
--- a/spec/unit/interface/config_spec.rb
+++ b/spec/unit/interface/config_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/config'
 
-describe Puppet::Interface::Config do
+describe Puppet::Interface.interface(:config) do
   before do
-    @interface = Puppet::Interface::Config
+    @interface = Puppet::Interface.interface(:config)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/configurer_spec.rb b/spec/unit/interface/configurer_spec.rb
index 99f5f7ad7..4b3532c1b 100644
--- a/spec/unit/interface/configurer_spec.rb
+++ b/spec/unit/interface/configurer_spec.rb
@@ -5,7 +5,7 @@ require 'puppet/interface/configurer'
 require 'puppet/indirector/catalog/rest'
 require 'tempfile'
 
-describe Puppet::Interface::Configurer do
+describe Puppet::Interface.interface(:configurer) do
   describe "#synchronize" do
     it "should retrieve and apply a catalog and return a report" do
       dirname = Dir.mktmpdir("puppetdir")
@@ -16,7 +16,7 @@ describe Puppet::Interface::Configurer do
       @catalog.add_resource(@file)
       Puppet::Resource::Catalog::Rest.any_instance.stubs(:find).returns(@catalog)
 
-      report = Puppet::Interface::Configurer.synchronize("foo")
+      report = Puppet::Interface.interface(:configurer).synchronize("foo")
 
       report.kind.should   == "apply"
       report.status.should == "changed"
diff --git a/spec/unit/interface/facts_spec.rb b/spec/unit/interface/facts_spec.rb
index c311b5d3d..03d6410f9 100644
--- a/spec/unit/interface/facts_spec.rb
+++ b/spec/unit/interface/facts_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/facts'
 
-describe Puppet::Interface::Facts do
+describe Puppet::Interface.interface(:facts) do
   before do
-    @interface = Puppet::Interface::Facts
+    @interface = Puppet::Interface.interface(:facts)
   end
 
   it "should define an 'upload' fact" do
diff --git a/spec/unit/interface/file_spec.rb b/spec/unit/interface/file_spec.rb
index 1d9e55752..fc7accf0d 100644
--- a/spec/unit/interface/file_spec.rb
+++ b/spec/unit/interface/file_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/file'
 
-describe Puppet::Interface::File do
+describe Puppet::Interface.interface(:file) do
   before do
-    @interface = Puppet::Interface::File
+    @interface = Puppet::Interface.interface(:file)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/key_spec.rb b/spec/unit/interface/key_spec.rb
index 9be024445..93a7c937c 100644
--- a/spec/unit/interface/key_spec.rb
+++ b/spec/unit/interface/key_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/key'
 
-describe Puppet::Interface::Key do
+describe Puppet::Interface.interface(:key) do
   before do
-    @interface = Puppet::Interface::Key
+    @interface = Puppet::Interface.interface(:key)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/node_spec.rb b/spec/unit/interface/node_spec.rb
index cfb15e38a..63109308d 100644
--- a/spec/unit/interface/node_spec.rb
+++ b/spec/unit/interface/node_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/node'
 
-describe Puppet::Interface::Node do
+describe Puppet::Interface.interface(:node) do
   before do
-    @interface = Puppet::Interface::Node
+    @interface = Puppet::Interface.interface(:node)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/report_spec.rb b/spec/unit/interface/report_spec.rb
index 932fc5cc9..b5bee1a62 100644
--- a/spec/unit/interface/report_spec.rb
+++ b/spec/unit/interface/report_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/report'
 
-describe Puppet::Interface::Report do
+describe Puppet::Interface.interface(:report) do
   before do
-    @interface = Puppet::Interface::Report
+    @interface = Puppet::Interface.interface(:report)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/resource_spec.rb b/spec/unit/interface/resource_spec.rb
index b28b04310..cad45b66b 100644
--- a/spec/unit/interface/resource_spec.rb
+++ b/spec/unit/interface/resource_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource'
 
-describe Puppet::Interface::Resource do
+describe Puppet::Interface.interface(:resource) do
   before do
-    @interface = Puppet::Interface::Resource
+    @interface = Puppet::Interface.interface(:resource)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface/resource_type_spec.rb b/spec/unit/interface/resource_type_spec.rb
index 95b9ec7f1..6c437c475 100644
--- a/spec/unit/interface/resource_type_spec.rb
+++ b/spec/unit/interface/resource_type_spec.rb
@@ -3,9 +3,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource_type'
 
-describe Puppet::Interface::ResourceType do
+describe Puppet::Interface.interface(:resource_type) do
   before do
-    @interface = Puppet::Interface::ResourceType
+    @interface = Puppet::Interface.interface(:resource_type)
   end
 
   it "should be a subclass of 'Indirection'" do
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 73f7a4569..876c7945c 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -30,8 +30,8 @@ describe Puppet::Interface do
     end
   end
 
-  it "should use its name converted to a string as its string form" do
-    Puppet::Interface.new(:me).to_s.should == "me"
+  it "should stringify with its own name" do
+    Puppet::Interface.new(:me).to_s.should =~ /\bme\b/
   end
 
   it "should allow overriding of the default format" do
@@ -53,48 +53,34 @@ describe Puppet::Interface do
     Puppet::Interface.new(:me, :verb => "foo").verb.should == "foo"
   end
 
-  it "should create an associated constant when registering an interface" do
-    $stderr.stubs(:puts)
-    face = Puppet::Interface.new(:me)
-    Puppet::Interface.register_interface(:me, face)
-    Puppet::Interface::Me.should equal(face)
-  end
-
   it "should try to require interfaces that are not known" do
     Puppet::Interface.expects(:require).with "puppet/interface/foo"
-    Puppet::Interface.const_get(:Foo)
-  end
-
-  it "should not fail when requiring an interface fails" do
-    $stderr.stubs(:puts)
-    Puppet::Interface.expects(:require).with("puppet/interface/foo").raises LoadError
-    lambda { Puppet::Interface::Foo }.should_not raise_error
+    Puppet::Interface.interface(:foo)
   end
 
   it "should be able to load all actions in all search paths"
 
-  describe "#constantize" do
+  describe "#underscorize" do
     faulty = [1, "#foo", "$bar", "sturm und drang", :"sturm und drang"]
     valid  = {
-      "foo"      => "Foo",
-      :foo       => "Foo",
-      "foo_bar"  => "FooBar",
-      :foo_bar   => "FooBar",
-      "foo-bar"  => "FooBar",
-      :"foo-bar" => "FooBar",
+      "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}' to '#{expect}'" do
-        result = Puppet::Interface.constantize(input)
-        result.should be_a String
-        result.to_s.should == expect
+      it "should map #{input.inspect} to #{expect.inspect}" do
+        result = Puppet::Interface.underscorize(input)
+        result.should == expect
       end
     end
 
     faulty.each do |input|
       it "should fail when presented with #{input.inspect} (#{input.class})" do
-        expect { Puppet::Interface.constantize(input) }.
+        expect { Puppet::Interface.underscorize(input) }.
           should raise_error ArgumentError, /not a valid interface name/
       end
     end