The first pass where at least all of the snippet tests pass. I have unfortunately had to stop being so assiduous in my rewriting of tests, but I am in too much of a time crunch to do this "right". The basic structure is definitely in place, though, and from here it is a question of making the rest of the tests work and hopefully writing some sufficient new tests, rather than making the code itself work.

2 files changed, 62 insertions, 137 deletions

diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
index c1c77a357..f1a20ee74 100644
--- a/lib/puppet/network/handler/configuration.rb
+++ b/lib/puppet/network/handler/configuration.rb
@@ -46,6 +46,8 @@ class Puppet::Network::Handler
             # immediately.  Mostly, this is so we can create the interpreter
             # on-demand, which is easier for testing.
             @options = options
+
+            set_server_facts
         end
 
         # Are we running locally, or are our clients networked?
@@ -58,7 +60,7 @@ class Puppet::Network::Handler
             v = interpreter.configuration_version
             # If we can find the node, then store the fact that the node
             # has checked in.
-            if node = node_handler.details(client)
+            if client and node = node_handler.details(client)
                 update_node_check(node)
             end
 
@@ -95,6 +97,9 @@ class Puppet::Network::Handler
                 begin
                     config = interpreter.compile(node)
                 rescue Puppet::Error => detail
+                    if Puppet[:trace]
+                        puts detail.backtrace
+                    end
                     Puppet.err detail
                     raise XMLRPC::FaultException.new(
                         1, detail.to_s
diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
index e889c1ba8..0aa50a426 100644
--- a/lib/puppet/network/handler/master.rb
+++ b/lib/puppet/network/handler/master.rb
@@ -21,66 +21,9 @@ class Puppet::Network::Handler
                 iface.add_method("int freshness()")
         }
 
-        # FIXME At some point, this should be autodocumenting.
-        def addfacts(facts)
-            # Add our server version to the fact list
-            facts["serverversion"] = Puppet.version.to_s
-
-            # And then add the server name and IP
-            {"servername" => "fqdn",
-                "serverip" => "ipaddress"
-            }.each do |var, fact|
-                if obj = Facter[fact]
-                    facts[var] = obj.value
-                else
-                    Puppet.warning "Could not retrieve fact %s" % fact
-                end
-            end
-
-            if facts["servername"].nil?
-                host = Facter.value(:hostname)
-                if domain = Facter.value(:domain)
-                    facts["servername"] = [host, domain].join(".")
-                else
-                    facts["servername"] = host
-                end
-            end
-        end
-
-        # Manipulate the client name as appropriate.
-        def clientname(name, ip, facts)
-            # Always use the hostname from Facter.
-            client = facts["hostname"]
-            clientip = facts["ipaddress"]
-            if Puppet[:node_name] == 'cert'
-                if name
-                    client = name
-                end
-                if ip
-                    clientip = ip
-                end
-            end
-
-            return client, clientip
-        end
-
         # Tell a client whether there's a fresh config for it
         def freshness(client = nil, clientip = nil)
-            if Puppet.features.rails? and Puppet[:storeconfigs]
-                Puppet::Rails.connect
-
-                host = Puppet::Rails::Host.find_or_create_by_name(client)
-                host.last_freshcheck = Time.now
-                if clientip and (! host.ip or host.ip == "" or host.ip == "NULL")
-                    host.ip = clientip
-                end
-                host.save
-            end
-            if defined? @interpreter
-                return @interpreter.parsedate
-            else
-                return 0
-            end
+            config_handler.version(client)
         end
 
         def initialize(hash = {})
@@ -121,104 +64,81 @@ class Puppet::Network::Handler
                 args[:Classes] = hash[:Classes]
             end
 
-            @interpreter = Puppet::Parser::Interpreter.new(args)
+            @config_handler = Puppet::Network::Handler.handler(:configuration).new(args)
         end
 
+        # Call our various handlers; this handler is getting deprecated.
         def getconfig(facts, format = "marshal", client = nil, clientip = nil)
-            if @local
-                # we don't need to do anything, since we should already
-                # have raw objects
-                Puppet.debug "Our client is local"
-            else
-                Puppet.debug "Our client is remote"
-
-                # XXX this should definitely be done in the protocol, somehow
-                case format
-                when "marshal":
-                    Puppet.warning "You should upgrade your client.  'Marshal' will not be supported much longer."
-                    begin
-                        facts = Marshal::load(CGI.unescape(facts))
-                    rescue => detail
-                        raise XMLRPC::FaultException.new(
-                            1, "Could not rebuild facts"
-                        )
-                    end
-                when "yaml":
-                    begin
-                        facts = YAML.load(CGI.unescape(facts))
-                    rescue => detail
-                        raise XMLRPC::FaultException.new(
-                            1, "Could not rebuild facts"
-                        )
-                    end
-                else
-                    raise XMLRPC::FaultException.new(
-                        1, "Unavailable config format %s" % format
-                    )
-                end
-            end
-
+            facts = decode_facts(facts)
             client, clientip = clientname(client, clientip, facts)
 
-            # Add any server-side facts to our server.
-            addfacts(facts)
+            # Pass the facts to the fact handler
+            fact_handler.set(client, facts)
 
-            retobjects = nil
+            # And get the configuration from the config handler
+            return config_handler.configuration(client)
+        end
 
-            # This is hackish, but there's no "silence" option for benchmarks
-            # right now
-            if @local
-                #begin
-                    retobjects = @interpreter.run(client, facts)
-                #rescue Puppet::Error => detail
-                #    Puppet.err detail
-                #    raise XMLRPC::FaultException.new(
-                #        1, detail.to_s
-                #    )
-                #rescue => detail
-                #    Puppet.err detail.to_s
-                #    return ""
-                #end
+        def local?
+            if defined? @local and @local
+                return true
             else
-                benchmark(:notice, "Compiled configuration for %s" % client) do
-                    begin
-                        retobjects = @interpreter.run(client, facts)
-                    rescue Puppet::Error => detail
-                        Puppet.err detail
-                        raise XMLRPC::FaultException.new(
-                            1, detail.to_s
-                        )
-                    rescue => detail
-                        Puppet.err detail.to_s
-                        return ""
-                    end
+                return false
+            end
+        end
+
+        private
+
+        # Manipulate the client name as appropriate.
+        def clientname(name, ip, facts)
+            # Always use the hostname from Facter.
+            client = facts["hostname"]
+            clientip = facts["ipaddress"]
+            if Puppet[:node_name] == 'cert'
+                if name
+                    client = name
                 end
+                if ip
+                    clientip = ip
+                end
+            end
+
+            return client, clientip
+        end
+
+        def config_handler
+            unless defined? @config_handler
+                @config_handler = Puppet::Network::Handler.handler(:config).new :local => local?
             end
+            @config_handler
+        end
 
+        # 
+        def decode_facts(facts)
             if @local
-                return retobjects
+                # we don't need to do anything, since we should already
+                # have raw objects
+                Puppet.debug "Our client is local"
             else
-                str = nil
-                case format
-                when "marshal":
-                    str = Marshal::dump(retobjects)
-                when "yaml":
-                    str = retobjects.to_yaml(:UseBlock => true)
-                else
+                Puppet.debug "Our client is remote"
+
+                begin
+                    facts = YAML.load(CGI.unescape(facts))
+                rescue => detail
                     raise XMLRPC::FaultException.new(
-                        1, "Unavailable config format %s" % format
+                        1, "Could not rebuild facts"
                     )
                 end
-                return CGI.escape(str)
             end
+
+            return facts
         end
 
-        def local?
-            if defined? @local and @local
-                return true
-            else
-                return false
+        def fact_handler
+            unless defined? @fact_handler
+                @fact_handler = Puppet::Network::Handler.handler(:facts).new :local => local?
             end
+            @fact_handler
         end
     end
 end