Another intermediate commit. The node and fact classes are now functional and are used instead of the network handlers, which have been removed. There are some failing tests as a result, but I want to get this code committed before I massage the rest of the system to make it work again.

12 files changed, 188 insertions, 175 deletions

diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index b8690d7d5..704039abc 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -11,8 +11,20 @@ module Puppet::Indirector
     class Terminus
         require 'puppet/util/docs'
         extend Puppet::Util::Docs
+
+        class << self
+            attr_accessor :name, :indirection
+        end
+        def name
+            self.class.name
+        end
+        def indirection
+            self.class.indirection
+        end
     end
 
+    require 'puppet/indirector/indirection'
+
     # This handles creating the terminus classes.
     require 'puppet/util/classgen'
     extend Puppet::Util::ClassGen
@@ -38,13 +50,15 @@ module Puppet::Indirector
     # 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)
-        genclass(terminus,
+        klass = genclass(terminus,
             :prefix => indirection.to_s.capitalize,
             :hash => instance_hash(indirection),
             :attributes => options,
             :block => block,
             :parent => options[:parent] || Terminus
         )
+        klass.indirection = indirection
+        klass.name = terminus
     end
 
     # Retrieve a terminus class by indirection and name.
@@ -61,23 +75,14 @@ module Puppet::Indirector
     # +:to+: What parameter to use as the name of the indirection terminus.
     def indirects(indirection, options = {})
         if defined?(@indirection)
-            raise ArgumentError, "Already performing an indirection of %s; cannot redirect %s" % [@indirection[:name], indirection]
+            raise ArgumentError, "Already performing an indirection of %s; cannot redirect %s" % [@indirection.name, indirection]
         end
-        options[:name] = indirection
-        @indirection = options
+        @indirection = Indirection.new(indirection, options)
 
-        # Validate the parameter.  This requires that indirecting
-        # classes require 'puppet/defaults', because of ordering issues,
-        # but it makes problems much easier to debug.
-        if param_name = options[:to]
-            begin
-                name = Puppet[param_name]
-            rescue
-                raise ArgumentError, "Configuration parameter '%s' for indirection '%s' does not exist'" % [param_name, indirection]
-            end
-        end
         # Set up autoloading of the appropriate termini.
         Puppet::Indirector.register_indirection indirection
+
+        return @indirection
     end
 
     # Define methods for each of the HTTP methods.  These just point to the
@@ -98,40 +103,16 @@ module Puppet::Indirector
 
     private
 
-
-    # Create a new terminus instance.
-    def make_terminus(name)
-        # Load our terminus class.
-        unless klass = Puppet::Indirector.terminus(@indirection[:name], name)
-            raise ArgumentError, "Could not find terminus %s for indirection %s" % [name, indirection]
-        end
-        return klass.new
-    end
-
     # Redirect a given HTTP method.
     def redirect(method_name, *args)
         begin
-            terminus.send(method_name, *args)
-        rescue NoMethodError
-            raise ArgumentError, "Indirection category %s does not respond to REST method %s" % [indirection, method_name]
-        end
-    end
-
-    # Return the singleton terminus for this indirection.
-    def terminus(name = nil)
-        @termini ||= {}
-        # Get the name of the terminus.
-        unless name
-            unless param_name = @indirection[:to]
-                raise ArgumentError, "You must specify an indirection terminus for indirection %s" % @indirection[:name]
+            @indirection.terminus.send(method_name, *args)
+        rescue NoMethodError => detail
+            if Puppet[:trace]
+                puts detail.backtrace
             end
-            name = Puppet[param_name]
-            name = name.intern if name.is_a?(String)
-        end
-        
-        unless @termini[name]
-            @termini[name] = make_terminus(name)
+            raise ArgumentError, "The %s terminus of the %s indirection failed to respond to %s: %s" %
+                [@indirection.terminus.name, @indirection.name, method_name, detail]
         end
-        @termini[name]
     end
 end
diff --git a/lib/puppet/indirector/facts/yaml.rb b/lib/puppet/indirector/facts/yaml.rb
index 87860012f..f29ea8ebc 100644
--- a/lib/puppet/indirector/facts/yaml.rb
+++ b/lib/puppet/indirector/facts/yaml.rb
@@ -21,7 +21,7 @@ Puppet::Indirector.register_terminus :facts, :yaml do
     end
 
     # Store the facts to disk.
-    def put(facts)
+    def post(facts)
         File.open(path(facts.name), "w", 0600) do |f|
             begin
                 f.print YAML::dump(facts.values)
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
new file mode 100644
index 000000000..7a4c4bd55
--- /dev/null
+++ b/lib/puppet/indirector/indirection.rb
@@ -0,0 +1,74 @@
+# An actual indirection.
+class Puppet::Indirector::Indirection
+    @@indirections = []
+
+    # Clear all cached termini from all indirections.
+    def self.clear_cache
+        @@indirections.each { |ind| ind.clear_cache }
+    end
+
+    attr_accessor :name, :termini
+    attr_reader :to
+
+    # Clear our cached list of termini.
+    # This is only used for testing.
+    def clear_cache
+        @termini.clear
+    end
+
+    # This is only used for testing.
+    def delete
+        @@indirections.delete(self) if @@indirections.include?(self)
+    end
+
+    def initialize(name, options = {})
+        @name = name
+        options.each do |name, value|
+            begin
+                send(name.to_s + "=", value)
+            rescue NoMethodError
+                raise ArgumentError, "%s is not a valid Indirection parameter" % name
+            end
+        end
+        @termini = {}
+        @@indirections << self
+    end
+
+    # Return the singleton terminus for this indirection.
+    def terminus(name = nil)
+        # Get the name of the terminus.
+        unless name
+            unless param_name = self.to
+                raise ArgumentError, "You must specify an indirection terminus for indirection %s" % self.name
+            end
+            name = Puppet[param_name]
+            name = name.intern if name.is_a?(String)
+        end
+        
+        unless @termini[name]
+            @termini[name] = make_terminus(name)
+        end
+        @termini[name]
+    end
+
+    # Validate the parameter.  This requires that indirecting
+    # classes require 'puppet/defaults', because of ordering issues,
+    # but it makes problems much easier to debug.
+    def to=(param_name)
+        unless Puppet.config.valid?(param_name)
+            raise ArgumentError, "Configuration parameter '%s' for indirection '%s' does not exist'" % [param_name, self.name]
+        end
+        @to = param_name
+    end
+
+    private
+
+    # Create a new terminus instance.
+    def make_terminus(name)
+        # Load our terminus class.
+        unless klass = Puppet::Indirector.terminus(self.name, name)
+            raise ArgumentError, "Could not find terminus %s for indirection %s" % [name, self.name]
+        end
+        return klass.new
+    end
+end
diff --git a/lib/puppet/indirector/node/external.rb b/lib/puppet/indirector/node/external.rb
index ed2a8893e..13cd265fb 100644
--- a/lib/puppet/indirector/node/external.rb
+++ b/lib/puppet/indirector/node/external.rb
@@ -1,18 +1,66 @@
+require 'puppet/node/facts'
+
 Puppet::Indirector.register_terminus :node, :external do
     desc "Call an external program to get node information."
 
     include Puppet::Util
+
+    # Proxy the execution, so it's easier to test.
+    def execute(command)
+        Puppet::Util.execute(command)
+    end
+
     # Look for external node definitions.
     def get(name)
-        return nil unless Puppet[:external_nodes] != "none"
+        unless Puppet[:external_nodes] != "none"
+            raise ArgumentError, "You must set the 'external_nodes' parameter to use the external node source"
+        end
   
+        unless Puppet[:external_nodes][0] == File::SEPARATOR[0]
+            raise ArgumentError, "You must set the 'external_nodes' parameter to a fully qualified command"
+        end
+
+        # Run the command.
+        unless output = query(name)
+            return nil
+        end
+
+        # Translate the output to ruby.
+        result = translate(name, output)
+
+        return create_node(name, result)
+    end
+
+    private
+
+    # Turn our outputted objects into a Puppet::Node instance.
+    def create_node(name, result)
+        node = Puppet::Node.new(name)
+        set = false
+        [:parameters, :classes].each do |param|
+            if value = result[param]
+                node.send(param.to_s + "=", value)
+                set = true
+            end
+        end
+
+        if set
+            node.fact_merge
+            return node
+        else
+            return nil
+        end
+    end
+
+    # Call the external command and see if it returns our output.
+    def query(name)
         # This is a very cheap way to do this, since it will break on
         # commands that have spaces in the arguments.  But it's good
         # enough for most cases.
         external_node_command = Puppet[:external_nodes].split
         external_node_command << name
         begin
-            output = Puppet::Util.execute(external_node_command)
+            output = execute(external_node_command)
         rescue Puppet::ExecutionFailure => detail
             if $?.exitstatus == 1
                 return nil
@@ -25,31 +73,17 @@ Puppet::Indirector.register_terminus :node, :external do
         if output =~ /\A\s*\Z/ # all whitespace
             Puppet.debug "Empty response for %s from external node source" % name
             return nil
+        else
+            return output
         end
+    end
 
+    # Translate the yaml string into Ruby objects.
+    def translate(name, output)
         begin
-            result = YAML.load(output).inject({}) { |hash, data| hash[symbolize(data[0])] = data[1]; hash }
+            YAML.load(output).inject({}) { |hash, data| hash[symbolize(data[0])] = data[1]; hash }
         rescue => detail
             raise Puppet::Error, "Could not load external node results for %s: %s" % [name, detail]
         end
-
-        node = Puppe::Node.new(name)
-        set = false
-        [:parameters, :classes].each do |param|
-            if value = result[param]
-                node.send(param.to_s + "=", value)
-                set = true
-            end
-        end
-
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
-
-        if set
-            return node
-        else
-            return nil
-        end
     end
 end
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 77be04126..fb60cad31 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -18,10 +18,8 @@ Puppet::Indirector.register_terminus :node, :ldap do
             end
         end
 
-        node = Puppe::Node.new(name, :classes => classes, :source => "ldap", :parameters => parameters)
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
+        node = Puppet::Node.new(name, :classes => classes, :source => "ldap", :parameters => parameters)
+        node.fact_merge
         return node
     end
 
diff --git a/lib/puppet/indirector/node/none.rb b/lib/puppet/indirector/node/none.rb
index 7143033d9..2b326968e 100644
--- a/lib/puppet/indirector/node/none.rb
+++ b/lib/puppet/indirector/node/none.rb
@@ -1,3 +1,5 @@
+require 'puppet/node/facts'
+
 Puppet::Indirector.register_terminus :node, :none do
     desc "Always return an empty node object.  This is the node source you should
         use when you don't have some other, functional source you want to use,
@@ -6,9 +8,7 @@ Puppet::Indirector.register_terminus :node, :none do
     # Just return an empty node.
     def get(name)
         node = Puppet::Node.new(name)
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
+        node.fact_merge
         node
     end
 end
diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
index 372e80325..2df1b3ab4 100644
--- a/lib/puppet/network/handler/configuration.rb
+++ b/lib/puppet/network/handler/configuration.rb
@@ -79,7 +79,7 @@ class Puppet::Network::Handler
         # Add any extra data necessary to the node.
         def add_node_data(node)
             # Merge in our server-side facts, so they can be used during compilation.
-            node.fact_merge(@server_facts)
+            node.merge(@server_facts)
 
             # Add any specified classes to the node's class list.
             if classes = @options[:Classes]
diff --git a/lib/puppet/network/handler/facts.rb b/lib/puppet/network/handler/facts.rb
deleted file mode 100755
index 4767e8be4..000000000
--- a/lib/puppet/network/handler/facts.rb
+++ /dev/null
@@ -1,68 +0,0 @@
-require 'yaml'
-require 'puppet/util/fact_store'
-
-class Puppet::Network::Handler
-    # Receive logs from remote hosts.
-    class Facts < Handler
-        desc "An interface for storing and retrieving client facts.  Currently only
-        used internally by Puppet."
-
-        @interface = XMLRPC::Service::Interface.new("facts") { |iface|
-            iface.add_method("void set(string, string)")
-            iface.add_method("string get(string)")
-            iface.add_method("integer store_date(string)")
-        }
-
-        def initialize(hash = {})
-            super
-
-            backend = Puppet[:factstore]
-
-            unless klass = Puppet::Util::FactStore.store(backend)
-                raise Puppet::Error, "Could not find fact store %s" % backend
-            end
-
-            @backend = klass.new
-        end
-
-        # Get the facts from our back end.
-        def get(node)
-            if facts = @backend.get(node)
-                return strip_internal(facts)
-            else
-                return nil
-            end
-        end
-
-        # Set the facts in the backend.
-        def set(node, facts)
-            @backend.set(node, add_internal(facts))
-            nil
-        end
-
-        # Retrieve a client's storage date.
-        def store_date(node)
-            if facts = get(node)
-                facts[:_puppet_timestamp].to_i
-            else
-                nil
-            end
-        end
-
-        private
-
-        # Add internal data to the facts for storage.
-        def add_internal(facts)
-            facts = facts.dup
-            facts[:_puppet_timestamp] = Time.now
-            facts
-        end
-
-        # Strip out that internal data.
-        def strip_internal(facts)
-            facts = facts.dup
-            facts.find_all { |name, value| name.to_s =~ /^_puppet_/ }.each { |name, value| facts.delete(name) }
-            facts
-        end
-    end
-end
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index a429412d2..993e9d51a 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -243,7 +243,7 @@ class Puppet::Network::Handler
         # the modules.
         def modules_mount(module_name, client)
             # Find our environment, if we have one.
-            if node = node_handler.details(client || Facter.value("hostname"))
+            if node = Puppet::Node.get(client || Facter.value("hostname"))
                 env = node.environment
             else
                 env = nil
@@ -258,14 +258,6 @@ class Puppet::Network::Handler
             end
         end
 
-        # Create a node handler instance for looking up our nodes.
-        def node_handler
-            unless defined?(@node_handler)
-                @node_handler = Puppet::Network::Handler.handler(:node).create
-            end
-            @node_handler
-        end
-
         # Read the configuration file.
         def readconfig(check = true)
             return if @noreadconfig
diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
index c8db277ba..9550dd550 100644
--- a/lib/puppet/network/handler/master.rb
+++ b/lib/puppet/network/handler/master.rb
@@ -74,7 +74,7 @@ class Puppet::Network::Handler
             client, clientip = clientname(client, clientip, facts)
 
             # Pass the facts to the fact handler
-            fact_handler.set(client, facts)
+            Puppet::Node::Facts.post(Puppet::Node::Facts.new(client, facts))
 
             # And get the configuration from the config handler
             begin
@@ -134,13 +134,6 @@ class Puppet::Network::Handler
             return facts
         end
 
-        def fact_handler
-            unless defined? @fact_handler
-                @fact_handler = Puppet::Network::Handler.handler(:facts).new :local => local?
-            end
-            @fact_handler
-        end
-
         # Translate our configuration appropriately for sending back to a client.
         def translate(config)
             if local?
diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb
index 9c5d2d397..7ad7bc3b3 100644
--- a/lib/puppet/node.rb
+++ b/lib/puppet/node.rb
@@ -1,8 +1,11 @@
+require 'puppet/indirector'
+
 # A simplistic class for managing the node information itself.
 class Puppet::Node
+    require 'puppet/node/facts'
+
     # Set up indirection, so that nodes can be looked for in
     # the node sources.
-    require 'puppet/indirector'
     extend Puppet::Indirector
 
     # Use the node source as the indirection terminus.
@@ -66,9 +69,15 @@ class Puppet::Node
     end
 
     # Merge the node facts with parameters from the node source.
-    # This is only called if the node source has 'fact_merge' set to true.
-    def fact_merge(facts)
-        facts.each do |name, value|
+    def fact_merge
+        if facts = Puppet::Node::Facts.get(name)
+            merge(facts.values)
+        end
+    end
+
+    # Merge any random parameters into our parameter list.
+    def merge(params)
+        params.each do |name, value|
             @parameters[name] = value unless @parameters.include?(name)
         end
     end
diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb
index eddf44def..e5774127b 100755
--- a/lib/puppet/node/facts.rb
+++ b/lib/puppet/node/facts.rb
@@ -1,9 +1,11 @@
+require 'puppet/node'
+require 'puppet/indirector'
+
 # Manage a given node's facts.  This either accepts facts and stores them, or
 # returns facts for a given node.
 class Puppet::Node::Facts
     # Set up indirection, so that nodes can be looked for in
     # the node sources.
-    require 'puppet/indirector'
     extend Puppet::Indirector
 
     # Use the node source as the indirection terminus.
@@ -14,23 +16,21 @@ class Puppet::Node::Facts
     def initialize(name, values = {})
         @name = name
         @values = values
+
+        add_internal
     end
 
     private
 
-    # FIXME These methods are currently unused.
-
     # Add internal data to the facts for storage.
-    def add_internal(facts)
-        facts = facts.dup
-        facts[:_puppet_timestamp] = Time.now
-        facts
+    def add_internal
+        self.values[:_timestamp] = Time.now
     end
 
     # Strip out that internal data.
-    def strip_internal(facts)
-        facts = facts.dup
-        facts.find_all { |name, value| name.to_s =~ /^_puppet_/ }.each { |name, value| facts.delete(name) }
-        facts
+    def strip_internal
+        newvals = values.dup
+        newvals.find_all { |name, value| name.to_s =~ /^_/ }.each { |name, value| newvals.delete(name) }
+        newvals
     end
 end