It looks like the new indirection setup is complete.

I only need to port the node indirection termini over.

6 files changed, 71 insertions, 68 deletions

diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index 4d7d9d92d..b89017c9a 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -10,53 +10,6 @@ module Puppet::Indirector
     require 'puppet/indirector/indirection'
     require 'puppet/indirector/terminus'
 
-    # This handles creating the terminus classes.
-    require 'puppet/util/classgen'
-    extend Puppet::Util::ClassGen
-
-    # This manages reading in all of our files for us and then retrieving
-    # loaded instances.  We still have to define the 'newX' method, but this
-    # does all of the rest -- loading, storing, and retrieving by name.
-    require 'puppet/util/instance_loader'
-    extend Puppet::Util::InstanceLoader
-    
-    # Register a given indirection type.  The classes including this module
-    # handle creating terminus instances, but the module itself handles
-    # loading them and managing the classes.
-    def self.enable_autoloading_indirection(indirection)
-        # Set up autoloading of the appropriate termini.
-        instance_load indirection, "puppet/indirector/%s" % indirection
-    end
-    
-# JRB:TODO -- where did this come from, re: the specs? also, any way to make this protected/private?
-    
-    # Define a new indirection terminus.  This method is used by the individual
-    # termini in their separate files.  Again, the autoloader takes care of
-    # actually loading these files.
-    #   Note that the termini are being registered on the Indirector module, not
-    # on the classes including the module.  This allows a given indirection to
-    # be used in multiple classes.
-    def self.register_terminus(indirection, terminus, options = {}, &block)
-        klass = genclass(terminus,
-            :prefix => indirection.to_s.capitalize,
-            :hash => instance_hash(indirection),
-            :attributes => options,
-            :block => block,
-            :parent => options[:parent] || Terminus,
-# JRB:FIXME -- why do I have to use overwrite here?            
-            :overwrite => 'please do motherfucker'
-        )
-        klass.indirection = indirection
-        klass.name = terminus
-    end
-
-# JRB:TODO where did this come from, re: the specs?  also, shouldn't this be protected/private?    
-    # Retrieve a terminus class by indirection and name.
-# JRB:FIXME -- should be protected/private
-    def self.terminus(indirection, terminus)
-        loaded_instance(indirection, terminus)
-    end
-
     # Declare that the including class indirects its methods to
     # this terminus.  The terminus name must be the name of a Puppet
     # default, not the value -- if it's the value, then it gets
@@ -70,7 +23,6 @@ module Puppet::Indirector
 
         # instantiate the actual Terminus for that type and this name (:ldap, w/ args :node)
         # & hook the instantiated Terminus into this class (Node: @indirection = terminus)
-        Puppet::Indirector.enable_autoloading_indirection indirection
         @indirection = Puppet::Indirector::Indirection.new(indirection)
     end
 
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 0cf8c7a27..31b9d68a5 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -36,6 +36,7 @@ class Puppet::Indirector::Indirection
             end
         end
         @termini = {}
+        @terminus_types = {}
         raise(ArgumentError, "Indirection %s is already defined" % @name) if @@indirections.find { |i| i.name == @name }
         @@indirections << self
     end
@@ -81,7 +82,7 @@ class Puppet::Indirector::Indirection
     # Create a new terminus instance.
     def make_terminus(name)
         # Load our terminus class.
-        unless klass = Puppet::Indirector.terminus(self.name, name)
+        unless klass = Puppet::Indirector::Terminus.terminus_class(self.name, name)
             raise ArgumentError, "Could not find terminus %s for indirection %s" % [name, self.name]
         end
         return klass.new
diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb
index a98f6dff0..f3797f471 100644
--- a/lib/puppet/indirector/terminus.rb
+++ b/lib/puppet/indirector/terminus.rb
@@ -1,14 +1,23 @@
 require 'puppet/indirector'
 require 'puppet/indirector/indirection'
+require 'puppet/util/instance_loader'
 
 # A simple class that can function as the base class for indirected types.
 class Puppet::Indirector::Terminus
     require 'puppet/util/docs'
     extend Puppet::Util::Docs
-    
+
     class << self
-        attr_accessor :name
-        attr_reader :indirection
+        include Puppet::Util::InstanceLoader
+
+        attr_accessor :name, :terminus_type
+        attr_reader :abstract_terminus, :indirection
+
+        # Are we an abstract terminus type, rather than an instance with an
+        # associated indirection?
+        def abstract_terminus?
+            abstract_terminus
+        end
 
         # Look up the indirection if we were only provided a name.
         def indirection=(name)
@@ -30,27 +39,66 @@ class Puppet::Indirector::Terminus
                 raise ArgumentError, "Terminus subclasses must have associated constants"
             end
             names = longname.split("::")
+            name = names.pop.downcase.intern
 
-            # First figure out the indirection
-            shortname = names.pop.downcase.intern
-            subclass.indirection = shortname
+            subclass.name = name
 
-            if names.empty?
-                raise ArgumentError, "Terminus subclasses need to have their constants include the parent class for setting the terminus name"
+            # Short-circuit the abstract types, which are those that directly subclass
+            # the Terminus class.
+            if self == Puppet::Indirector::Terminus
+                subclass.mark_as_abstract_terminus
+                return
             end
 
-            # And then the name (e.g., ldap or yaml)
-            termtype = names.pop.downcase.intern
-            subclass.name = termtype
+            # Set the terminus type to be the name of the abstract terminus type.
+            # Yay, class/instance confusion.
+            subclass.terminus_type = self.name
+
+            # This will throw an exception if the indirection instance cannot be found.
+            # Do this last, because it also registers the terminus type with the indirection,
+            # which needs the above information.
+            subclass.indirection = name
+
+            # And add this instance to the instance hash.
+            Puppet::Indirector::Terminus.register_terminus_class(subclass)
+        end
+
+        # Mark that this instance is abstract.
+        def mark_as_abstract_terminus
+            @abstract_terminus = true
+        end
+
+        # Register a class, probably autoloaded.
+        def register_terminus_class(klass)
+            setup_instance_loading klass.terminus_type
+            instance_hash(klass.terminus_type)[klass.name] = klass
+        end
+
+        # Return a terminus by name, using the autoloader.
+        def terminus_class(type, name)
+            setup_instance_loading type
+            loaded_instance(type, name)
+        end
+
+        private
+
+        def setup_instance_loading(type)
+            unless instance_loading?(type)
+                instance_load type, "puppet/indirector/%s" % type
+            end
         end
     end
 
     def initialize
-        unless indirection
-            raise Puppet::DevError, "Indirection termini cannot be used without an associated indirection"
+        if self.class.abstract_terminus?
+            raise Puppet::DevError, "Cannot create instances of abstract terminus types"
         end
     end
     
+    def terminus_type
+        self.class.terminus_type
+    end
+    
     def name
         self.class.name
     end
diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb
index f7f7d290f..42f1bb703 100644
--- a/lib/puppet/indirector/yaml.rb
+++ b/lib/puppet/indirector/yaml.rb
@@ -3,8 +3,6 @@ require 'puppet/indirector/terminus'
 # The base class for YAML indirection termini.
 class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
     def initialize
-        super
-
         # Make sure our base directory exists.
         Puppet.config.use(:yaml)
     end
@@ -42,6 +40,6 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
 
     # Return the path to a given node's file.
     def path(name)
-        File.join(Puppet[:yamldir], indirection.name.to_s, name.to_s + ".yaml")
+        File.join(Puppet[:yamldir], self.name.to_s, name.to_s + ".yaml")
     end
 end
diff --git a/lib/puppet/indirector/yaml/facts.rb b/lib/puppet/indirector/yaml/facts.rb
index c09eaeab1..754b0d5a6 100644
--- a/lib/puppet/indirector/yaml/facts.rb
+++ b/lib/puppet/indirector/yaml/facts.rb
@@ -2,6 +2,4 @@ require 'puppet/indirector/yaml'
 
 class Puppet::Indirector::Yaml::Facts < Puppet::Indirector::Yaml
     desc "Store client facts as flat files, serialized using YAML."
-
-    register_terminus
 end
diff --git a/lib/puppet/util/instance_loader.rb b/lib/puppet/util/instance_loader.rb
index bc0567866..f280014eb 100755
--- a/lib/puppet/util/instance_loader.rb
+++ b/lib/puppet/util/instance_loader.rb
@@ -5,6 +5,12 @@ require 'puppet/util'
 # of Puppet::Util::Autoload
 module Puppet::Util::InstanceLoader
     include Puppet::Util
+
+    # Are we instance-loading this type?
+    def instance_loading?(type)
+        defined?(@autoloaders) and @autoloaders.include?(symbolize(type))
+    end
+
     # Define a new type of autoloading.
     def instance_load(type, path, options = {})
         @autoloaders ||= {}
@@ -54,7 +60,7 @@ module Puppet::Util::InstanceLoader
     # Retrieve an alread-loaded instance, or attempt to load our instance.
     def loaded_instance(type, name)
         name = symbolize(name)
-        instances = instance_hash(type)
+        return nil unless instances = instance_hash(type)
         unless instances.include? name
             if instance_loader(type).load(name)
                 unless instances.include? name