Finalizing the node handler. It now correctly uses the different node sources

and knows how to retrieve data from those sources.  Now I just need to fix
the language stuff to use this handler instead of the existing node stuff.

7 files changed, 472 insertions, 318 deletions

diff --git a/lib/puppet/configuration.rb b/lib/puppet/configuration.rb
index 65e0d9fa8..574038338 100644
--- a/lib/puppet/configuration.rb
+++ b/lib/puppet/configuration.rb
@@ -122,7 +122,11 @@ module Puppet
             "The configuration file that defines the rights to the different
             namespaces and methods.  This can be used as a coarse-grained
             authorization system for both ``puppetd`` and ``puppetmasterd``."
-        ]
+        ],
+        :environment => ["", "The environment Puppet is running in.  For clients (e.g., ``puppetd``) this
+            determines the environment itself, which is used to find modules and much more.  For
+            servers (i.e., ``puppetmasterd``) this provides the default environment for nodes we
+            know nothing about."]
     )
 
     hostname = Facter["hostname"].value
@@ -544,7 +548,9 @@ module Puppet
 
     setdefaults(:parser,
         :typecheck => [true, "Whether to validate types during parsing."],
-        :paramcheck => [true, "Whether to validate parameters during parsing."]
+        :paramcheck => [true, "Whether to validate parameters during parsing."],
+        :node_source => ["", "Where to look for node configuration information.
+            See the `NodeSourceReference`:trac: for more information."]
     )
     
     setdefaults(:main,
diff --git a/lib/puppet/network/handler/node.rb b/lib/puppet/network/handler/node.rb
index 0c532144a..c9548f10b 100644
--- a/lib/puppet/network/handler/node.rb
+++ b/lib/puppet/network/handler/node.rb
@@ -9,12 +9,10 @@ require 'puppet/util/instance_loader'
 class Puppet::Network::Handler::Node < Puppet::Network::Handler
     # A simplistic class for managing the node information itself.
     class SimpleNode
-        attr_accessor :name, :classes, :parameters, :environment
+        attr_accessor :name, :classes, :parameters, :environment, :source
 
-        def initialize(options)
-            unless @name = options[:name]
-                raise ArgumentError, "Nodes require names" unless self.name
-            end
+        def initialize(name, options = {})
+            @name = name
 
             if classes = options[:classes]
                 if classes.is_a?(String)
@@ -27,19 +25,35 @@ class Puppet::Network::Handler::Node < Puppet::Network::Handler
             end
 
             @parameters = options[:parameters] || {}
+
+            unless @environment = options[:environment] 
+                if env = Puppet[:environment] and env != ""
+                    @environment = env
+                end
+            end
+        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|
+                @parameters[name] = value unless @parameters.include?(name)
+            end
         end
     end
+
     desc "Retrieve information about nodes."
 
     extend Puppet::Util::ClassGen
     extend Puppet::Util::InstanceLoader
 
+    # A simple base module we can use for modifying how our node sources work.
     module SourceBase
         include Puppet::Util::Docs
     end
 
     @interface = XMLRPC::Service::Interface.new("nodes") { |iface|
-        iface.add_method("string node(key)")
+        iface.add_method("string details(key)")
         iface.add_method("string parameters(key)")
         iface.add_method("string environment(key)")
         iface.add_method("string classes(key)")
@@ -48,11 +62,18 @@ class Puppet::Network::Handler::Node < Puppet::Network::Handler
     # Set up autoloading and retrieving of reports.
     autoload :node_source, 'puppet/node_source'
 
+    attr_reader :source
+
     # Add a new node source.
     def self.newnode_source(name, options = {}, &block)
         name = symbolize(name)
 
-        genmodule(name, :extend => SourceBase, :hash => instance_hash(:node_source), :block => block)
+        fact_merge = options[:fact_merge]
+        mod = genmodule(name, :extend => SourceBase, :hash => instance_hash(:node_source), :block => block)
+        mod.send(:define_method, :fact_merge?) do
+            fact_merge
+        end
+        mod
     end
 
     # Collect the docs for all of our node sources.
@@ -79,68 +100,141 @@ class Puppet::Network::Handler::Node < Puppet::Network::Handler
 
     # Remove a defined node source; basically only used for testing.
     def self.rm_node_source(name)
-        instance_hash(:node_source).delete(name)
+        rmclass(name, :hash => instance_hash(:node_source))
     end
 
     # Return a given node's classes.
     def classes(key)
-        raise "look up classes"
-    end
-
-    # Return a given node's environment.
-    def environment(key)
-        raise "look up environment"
-        if node = node(key)
-            node.environment
+        if node = details(key)
+            node.classes
         else
             nil
         end
     end
 
-    # Return an entire node configuration.
-    def node(key)
-        # Try to find our node...
-        nodes = nodes.collect { |n| n.to_s.downcase }
-
-        method = "nodesearch_%s" % @nodesource
-        # Do an inverse sort on the length, so the longest match always
-        # wins
-        nodes.sort { |a,b| b.length <=> a.length }.each do |node|
-            node = node.to_s if node.is_a?(Symbol)
-            if obj = self.send(method, node)
-                if obj.is_a?(AST::Node)
-                    nsource = obj.file
-                else
-                    nsource = obj.source
-                end
-                Puppet.info "Found %s in %s" % [node, nsource]
-                return obj
+    # Return an entire node configuration.  This uses the 'nodesearch' method
+    # defined in the node_source to look for the node.
+    def details(key)
+        facts = node_facts(key)
+        node = nil
+        names = node_names(key, facts)
+        names.each do |name|
+            name = name.to_s if name.is_a?(Symbol)
+            if node = nodesearch(name)
+                Puppet.info "Found %s in %s" % [name, @source]
+                break
             end
         end
 
         # If they made it this far, we haven't found anything, so look for a
         # default node.
-        unless nodes.include?("default")
-            if defobj = self.nodesearch("default")
-                Puppet.notice "Using default node for %s" % [nodes[0]]
-                return defobj
+        unless node or names.include?("default")
+            if node = nodesearch("default")
+                Puppet.notice "Using default node for %s" % key
+            end
+        end
+
+        if node
+            node.source = @source
+
+            # Merge the facts into the parameters.
+            if fact_merge?
+                node.fact_merge(facts)
             end
+            return node
+        else
+            return nil
+        end
+    end
+
+    # Return a given node's environment.
+    def environment(key)
+        if node = details(key)
+            node.environment
+        else
+            nil
         end
+    end
+
+    # Create our node lookup tool.
+    def initialize(hash = {})
+        @source = hash[:Source] || Puppet[:node_source]
+
+        unless mod = self.class.node_source(@source)
+            raise ArgumentError, "Unknown node source '%s'" % @source
+        end
+
+        extend(mod)
 
-        return nil
+        super
     end
 
+    # Short-hand for creating a new node, so the node sources don't need to
+    # specify the constant.
+    def newnode(options)
+        SimpleNode.new(options)
+    end
+
+    # Try to retrieve a given node's parameters.
     def parameters(key)
-        raise "Look up parameters"
+        if node = details(key)
+            node.parameters
+        else
+            nil
+        end
     end
 
     private
+
+    # Create/cache a fact handler.
+    def fact_handler
+        unless defined?(@fact_handler)
+            @fact_handler = Puppet::Network::Handler.handler(:facts).new
+        end
+        @fact_handler
+    end
+
+    # Look up the node facts from our fact handler.
     def node_facts(key)
-        raise "Look up node facts"
+        if facts = fact_handler.get(key)
+            facts
+        else
+            {}
+        end
     end
 
+    # Calculate the list of node names we should use for looking
+    # up our node.
     def node_names(key, facts = nil)
         facts ||= node_facts(key)
-        raise "Calculate node names"
+        names = []
+
+        if hostname = facts["hostname"]
+            unless hostname == key
+                names << hostname
+            end
+        else
+            hostname = key
+        end
+
+        if fqdn = facts["fqdn"]
+            hostname = fqdn
+            names << fqdn
+        end
+
+        # Make sure both the fqdn and the short name of the
+        # host can be used in the manifest
+        if hostname =~ /\./
+            names << hostname.sub(/\..+/,'')
+        elsif domain = facts['domain']
+            names << hostname + "." + domain
+        end
+
+        # Sort the names inversely by name length.
+        names.sort! { |a,b| b.length <=> a.length }
+
+        # And make sure the key is first, since that's the most
+        # likely usage.
+        ([key] + names).uniq
     end
 end
diff --git a/lib/puppet/node_source/external.rb b/lib/puppet/node_source/external.rb
index 4af68d8ae..54111d924 100644
--- a/lib/puppet/node_source/external.rb
+++ b/lib/puppet/node_source/external.rb
@@ -1,4 +1,4 @@
-Puppet::Network::Handler::Node.newnode_source(:external) do
+Puppet::Network::Handler::Node.newnode_source(:external, :fact_merge => true) do
     desc "Call an external program to get node information."
 
     include Puppet::Util
@@ -33,7 +33,7 @@ Puppet::Network::Handler::Node.newnode_source(:external) do
             raise Puppet::Error, "Could not load external node results for %s: %s" % [name, detail]
         end
 
-        node = Puppet::Network::Handler::Node::SimpleNode.new(:name => name)
+        node = newnode(name)
         set = false
         [:parameters, :classes].each do |param|
             if value = result[param]
diff --git a/lib/puppet/node_source/ldap.rb b/lib/puppet/node_source/ldap.rb
index 6825f2b68..9332fcb40 100644
--- a/lib/puppet/node_source/ldap.rb
+++ b/lib/puppet/node_source/ldap.rb
@@ -1,4 +1,4 @@
-Puppet::Network::Handler::Node.newnode_source(:ldap) do
+Puppet::Network::Handler::Node.newnode_source(:ldap, :fact_merge => true) do
     desc "Search in LDAP for node configuration information."
 
     # Find the ldap node, return the class list and parent node specially,
@@ -113,6 +113,6 @@ Puppet::Network::Handler::Node.newnode_source(:ldap) do
             end
         end
 
-        return Puppet::Network::Handler::Node::SimpleNode.new(:name => node, :classes => classes, :source => "ldap", :parameters => parameters)
+        return newnode(node, :classes => classes, :source => "ldap", :parameters => parameters)
     end
 end
diff --git a/lib/puppet/parser/node.rb b/lib/puppet/parser/node.rb
deleted file mode 100644
index c7979e51f..000000000
--- a/lib/puppet/parser/node.rb
+++ /dev/null
@@ -1,133 +0,0 @@
-#  Created by Luke A. Kanies on 2007-08-13.
-#  Copyright (c) 2007. All rights reserved.
-
-require 'puppet/external/gratr/digraph'
-require 'puppet/external/gratr/import'
-require 'puppet/external/gratr/dot'
-
-# Maintain a graph of scopes, along with a bunch of data
-# about the individual configuration we're compiling.
-class Puppet::Parser::Configuration
-    attr_reader :topscope, :interpreter, :host, :facts
-
-    # Add a collection to the global list.
-    def add_collection(coll)
-        @collections << coll
-    end
-
-    # Store the fact that we've evaluated a class, and store a reference to
-    # the scope in which it was evaluated, so that we can look it up later.
-    def class_set(name, scope)
-        @class_scopes[name] = scope
-    end
-
-    # Return the scope associated with a class.  This is just here so
-    # that subclasses can set their parent scopes to be the scope of
-    # their parent class, and it's also used when looking up qualified
-    # variables.
-    def class_scope(klass)
-        # They might pass in either the class or class name
-        if klass.respond_to?(:classname)
-            @class_scopes[klass.classname]
-        else
-            @class_scopes[klass]
-        end
-    end
-
-    # Return a list of all of the defined classes.
-    def classlist
-        return @class_scopes.keys.reject { |k| k == "" }
-    end
-
-    # Should the scopes behave declaratively?
-    def declarative?
-        true
-    end
-
-    # Set up our configuration.  We require an interpreter
-    # and a host name, and we normally are passed facts, too.
-    def initialize(options)
-        @interpreter = options[:interpreter] or
-            raise ArgumentError, "You must pass an interpreter to the configuration"
-        @facts = options[:facts] || {}
-        @host = options[:host] or
-            raise ArgumentError, "You must pass a host name to the configuration"
-
-        # Call the setup methods from the base class.
-        super()
-
-        initvars()
-    end
-
-    # Create a new scope, with either a specified parent scope or
-    # using the top scope.  Adds an edge between the scope and
-    # its parent to the graph.
-    def newscope(parent = nil)
-        parent ||= @topscope
-        scope = Puppet::Parser::Scope.new(:configuration => self)
-        @graph.add_edge!(parent, scope)
-        scope
-    end
-
-    # Find the parent of a given scope.  Assumes scopes only ever have
-    # one in edge, which will always be true.
-    def parent(scope)
-        if ary = @graph.adjacent(scope, :direction => :in) and ary.length > 0
-            ary[0]
-        else
-            nil
-        end
-    end
-
-    # Return an array of all of the unevaluated objects
-    def unevaluated
-        ary = @definedtable.find_all do |name, object|
-            ! object.builtin? and ! object.evaluated?
-        end.collect { |name, object| object }
-
-        if ary.empty?
-            return nil
-        else
-            return ary
-        end
-    end
-
-    private
-
-    # Set up all of our internal variables.
-    def initvars
-        # The table for storing class singletons.  This will only actually
-        # be used by top scopes and node scopes.
-        @class_scopes = {}
-
-        # The table for all defined resources.
-        @resource_table = {}
-
-        # The list of objects that will available for export.
-        @exported_resources = {}
-
-        # The list of overrides.  This is used to cache overrides on objects
-        # that don't exist yet.  We store an array of each override.
-        @resource_overrides = Hash.new do |overs, ref|
-            overs[ref] = []
-        end
-
-        # The list of collections that have been created.  This is a global list,
-        # but they each refer back to the scope that created them.
-        @collections = []
-
-        # Create our initial scope, our scope graph, and add the initial scope to the graph.
-        @topscope = Puppet::Parser::Scope.new(:configuration => self, :type => "main", :name => "top")
-        @graph = GRATR::Digraph.new
-        @graph.add_vertex!(@topscope)
-    end
-
-    # Return the list of remaining overrides.
-    def overrides
-        @resource_overrides.values.flatten
-    end
-
-    def resources
-        @resourcetable
-    end
-end
diff --git a/test/language/node.rb b/test/language/node.rb
deleted file mode 100755
index 4cbba8063..000000000
--- a/test/language/node.rb
+++ /dev/null
@@ -1,131 +0,0 @@
-#!/usr/bin/env ruby
-
-$:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/
-
-require 'mocha'
-require 'puppettest'
-require 'puppettest/parsertesting'
-require 'puppet/parser/configuration'
-
-# Test our configuration object.
-class TestConfiguration < Test::Unit::TestCase
-    include PuppetTest
-    include PuppetTest::ParserTesting
-
-    Config = Puppet::Parser::Configuration 
-    Scope = Puppet::Parser::Scope 
-
-    def mkconfig
-        Config.new(:host => "foo", :interpreter => "interp")
-    end
-
-    def test_initialize
-        # Make sure we get an error if we don't send an interpreter
-        assert_raise(ArgumentError, "Did not fail when missing host") do
-            Config.new(:interpreter => "yay" )
-        end
-        assert_raise(ArgumentError, "Did not fail when missing interp") do
-            Config.new(:host => "foo")
-        end
-
-        # Now check the defaults
-        config = nil
-        assert_nothing_raised("Could not init config with all required options") do
-            config = Config.new(:host => "foo", :interpreter => "interp")
-        end
-
-        assert_equal("foo", config.host, "Did not set host correctly")
-        assert_equal("interp", config.interpreter, "Did not set interpreter correctly")
-        assert_equal({}, config.facts, "Did not set default facts")
-
-        # Now make a new one with facts, to make sure the facts get set appropriately
-        assert_nothing_raised("Could not init config with all required options") do
-            config = Config.new(:host => "foo", :interpreter => "interp", :facts => {"a" => "b"})
-        end
-        assert_equal({"a" => "b"}, config.facts, "Did not set facts")
-    end
-
-    def test_initvars
-        config = mkconfig
-        [:class_scopes, :resource_table, :exported_resources, :resource_overrides].each do |table|
-            assert_instance_of(Hash, config.send(:instance_variable_get, "@#{table}"), "Did not set %s table correctly" % table)
-        end
-    end
-
-    # Make sure we store and can retrieve references to classes and their scopes.
-    def test_class_set_and_class_scope
-        klass = Object.new
-        klass.expects(:classname).returns("myname")
-
-        config = mkconfig
-        
-        assert_nothing_raised("Could not set class") do
-            config.class_set "myname", "myscope"
-        end
-        # First try to retrieve it by name.
-        assert_equal("myscope", config.class_scope("myname"), "Could not retrieve class scope by name")
-
-        # Then by object
-        assert_equal("myscope", config.class_scope(klass), "Could not retrieve class scope by object")
-    end
-    
-    def test_classlist
-        config = mkconfig
-
-        config.class_set "", "empty"
-        config.class_set "one", "yep"
-        config.class_set "two", "nope"
-
-        # Make sure our class list is correct
-        assert_equal(%w{one two}.sort, config.classlist.sort, "Did not get correct class list")
-    end
-
-    # Make sure collections get added to our internal array
-    def test_add_collection
-        config = mkconfig
-        assert_nothing_raised("Could not add collection") do
-            config.add_collection "nope"
-        end
-        assert_equal(%w{nope}, config.instance_variable_get("@collections"), "Did not add collection")
-    end
-
-    # Make sure we create a graph of scopes.
-    def test_newscope
-        config = mkconfig
-        graph = config.instance_variable_get("@graph")
-        assert_instance_of(Scope, config.topscope, "Did not create top scope")
-        assert_instance_of(GRATR::Digraph, graph, "Did not create graph")
-
-        assert(graph.vertex?(config.topscope), "The top scope is not a vertex in the graph")
-
-        # Now that we've got the top scope, create a new, subscope
-        subscope = nil
-        assert_nothing_raised("Could not create subscope") do
-            subscope = config.newscope
-        end
-        assert_instance_of(Scope, subscope, "Did not create subscope")
-        assert(graph.edge?(config.topscope, subscope), "An edge between top scope and subscope was not added")
-
-        # Make sure a scope can find its parent.
-        assert(config.parent(subscope), "Could not look up parent scope on configuration")
-        assert_equal(config.topscope.object_id, config.parent(subscope).object_id, "Did not get correct parent scope from configuration")
-        assert_equal(config.topscope.object_id, subscope.parent.object_id, "Scope did not correctly retrieve its parent scope")
-
-        # Now create another, this time specifying the parent scope
-        another = nil
-        assert_nothing_raised("Could not create subscope") do
-            another = config.newscope(subscope)
-        end
-        assert_instance_of(Scope, another, "Did not create second subscope")
-        assert(graph.edge?(subscope, another), "An edge between parent scope and second subscope was not added")
-
-        # Make sure it can find its parent.
-        assert(config.parent(another), "Could not look up parent scope of second subscope on configuration")
-        assert_equal(subscope.object_id, config.parent(another).object_id, "Did not get correct parent scope of second subscope from configuration")
-        assert_equal(subscope.object_id, another.parent.object_id, "Second subscope did not correctly retrieve its parent scope")
-
-        # And make sure both scopes show up in the right order in the search path
-        assert_equal([another.object_id, subscope.object_id, config.topscope.object_id], another.scope_path.collect { |p| p.object_id },
-            "Did not get correct scope path")
-    end
-end
diff --git a/test/network/handler/node.rb b/test/network/handler/node.rb
index c682929d9..02da9eef0 100755
--- a/test/network/handler/node.rb
+++ b/test/network/handler/node.rb
@@ -43,31 +43,342 @@ module NodeTesting
     def mk_searcher(name)
         searcher = Object.new
         searcher.extend(Node.node_source(name))
+        searcher.meta_def(:newnode) do |name, *args|
+            SimpleNode.new(name, *args)
+        end
+        searcher
+    end
+
+    def mk_node_source
+        @node_info = {}
+        @node_source = Node.newnode_source(:testing, :fact_merge => true) do
+            def nodesearch(key)
+                if info = @node_info[key]
+                    SimpleNode.new(info)
+                else
+                    nil
+                end
+            end
+        end
+        Puppet[:node_source] = "testing"
+
+        cleanup { Node.rm_node_source(:testing) }
     end
 end
 
 class TestNodeInterface < Test::Unit::TestCase
+    include NodeTesting
+
     def setup
         super
+        mk_node_source
+    end
+
+    # Make sure that the handler includes the appropriate
+    # node source.
+    def test_initialize
+        # First try it when passing in the node source
+        handler = nil
+        assert_nothing_raised("Could not specify a node source") do
+            handler = Node.new(:Source => :testing)
+        end
+        assert(handler.metaclass.included_modules.include?(@node_source), "Handler did not include node source")
+
+        # Now use the Puppet[:node_source]
+        Puppet[:node_source] = "testing"
+        assert_nothing_raised("Could not specify a node source") do
+            handler = Node.new()
+        end
+        assert(handler.metaclass.included_modules.include?(@node_source), "Handler did not include node source")
+
+        # And make sure we throw an exception when an invalid node source is used
+        assert_raise(ArgumentError, "Accepted an invalid node source") do
+            handler = Node.new(:Source => "invalid")
+        end
     end
 
-    def teardown
+    # Make sure we can find and we cache a fact handler.
+    def test_fact_handler
+        handler = Node.new
+        fhandler = nil
+        assert_nothing_raised("Could not retrieve the fact handler") do
+            fhandler = handler.send(:fact_handler)
+        end
+        assert_instance_of(Puppet::Network::Handler::Facts, fhandler, "Did not get a fact handler back")
+
+        # Now call it again, making sure we're caching the value.
+        fhandler2 = nil
+        assert_nothing_raised("Could not retrieve the fact handler") do
+            fhandler2 = handler.send(:fact_handler)
+        end
+        assert_instance_of(Puppet::Network::Handler::Facts, fhandler2, "Did not get a fact handler on the second run")
+        assert_equal(fhandler.object_id, fhandler2.object_id, "Did not cache fact handler")
     end
 
-    def test_node
-        raise "Failing, yo"
+    # Make sure we can get node facts from the fact handler.
+    def test_node_facts
+        # Check the case where we find the node.
+        handler = Node.new
+        fhandler = handler.send(:fact_handler)
+        fhandler.expects(:get).with("present").returns("a" => "b")
+
+        result = nil
+        assert_nothing_raised("Could not get facts from fact handler") do
+            result = handler.send(:node_facts, "present")
+        end
+        assert_equal({"a" => "b"}, result, "Did not get correct facts back")
+
+        # Now try the case where the fact handler knows nothing about our host
+        fhandler.expects(:get).with('missing').returns(nil)
+        result = nil
+        assert_nothing_raised("Could not get facts from fact handler when host is missing") do
+            result = handler.send(:node_facts, "missing")
+        end
+        assert_equal({}, result, "Did not get empty hash when no facts are known")
     end
 
+    # Test our simple shorthand
+    def test_newnode
+        SimpleNode.expects(:new).with("stuff")
+        handler = Node.new
+        handler.newnode("stuff")
+    end
+
+    # Make sure we can build up the correct node names to search for
+    def test_node_names
+        handler = Node.new
+
+        # Verify that the handler asks for the facts if we don't pass them in
+        handler.expects(:node_facts).with("testing").returns({})
+        handler.send(:node_names, "testing")
+
+        handler = Node.new
+        # Test it first with no parameters
+        assert_equal(%w{testing}, handler.send(:node_names, "testing"), "Node names did not default to an array including just the node name")
+
+        # Now test it with a fully qualified name
+        assert_equal(%w{testing.domain.com testing}, handler.send(:node_names, "testing.domain.com"),
+            "Fully qualified names did not get turned into multiple names, longest first")
+
+        # And try it with a short name + domain fact
+        assert_equal(%w{testing host.domain.com host}, handler.send(:node_names, "testing", "domain" => "domain.com", "hostname" => "host"),
+            "The domain fact was not used to build up an fqdn")
+
+        # And with an fqdn
+        assert_equal(%w{testing host.domain.com host}, handler.send(:node_names, "testing", "fqdn" => "host.domain.com"),
+            "The fqdn was not used")
+
+        # And make sure the fqdn beats the domain
+        assert_equal(%w{testing host.other.com host}, handler.send(:node_names, "testing", "domain" => "domain.com", "fqdn" => "host.other.com"),
+            "The domain was used in preference to the fqdn")
+    end
+
+    # Make sure we can retrieve a whole node by name.
+    def test_details_when_we_find_nodes
+        handler = Node.new
+
+        # Make sure we get the facts first
+        handler.expects(:node_facts).with("host").returns(:facts)
+
+        # Find the node names
+        handler.expects(:node_names).with("host", :facts).returns(%w{a b c})
+
+        # Iterate across them
+        handler.expects(:nodesearch).with("a").returns(nil)
+        handler.expects(:nodesearch).with("b").returns(nil)
+
+        # Create an example node to return
+        node = SimpleNode.new("host")
+
+        # Make sure its source is set
+        node.expects(:source=).with(handler.source)
+
+        # And make sure we actually get it back
+        handler.expects(:nodesearch).with("c").returns(node)
+
+        handler.expects(:fact_merge?).returns(true)
+
+        # Make sure we merge the facts with the node's parameters.
+        node.expects(:fact_merge).with(:facts)
+
+        # Now call the method
+        result = nil
+        assert_nothing_raised("could not call 'details'") do
+            result = handler.details("host")
+        end
+        assert_equal(node, result, "Did not get correct node back")
+    end
+
+    # But make sure we pass through to creating default nodes when appropriate.
+    def test_details_using_default_node
+        handler = Node.new
+
+        # Make sure we get the facts first
+        handler.expects(:node_facts).with("host").returns(:facts)
+
+        # Find the node names
+        handler.expects(:node_names).with("host", :facts).returns([])
+
+        # Create an example node to return
+        node = SimpleNode.new("host")
+
+        # Make sure its source is set
+        node.expects(:source=).with(handler.source)
+
+        # And make sure we actually get it back
+        handler.expects(:nodesearch).with("default").returns(node)
+
+        # This time, have it return false
+        handler.expects(:fact_merge?).returns(false)
+
+        # And because fact_merge was false, we don't merge them.
+        node.expects(:fact_merge).never
+
+        # Now call the method
+        result = nil
+        assert_nothing_raised("could not call 'details'") do
+            result = handler.details("host")
+        end
+        assert_equal(node, result, "Did not get correct node back")
+    end
+
+    # Make sure our handler behaves rationally when it comes to getting environment data.
     def test_environment
-        raise "still failing"
+        # What happens when we can't find the node
+        handler = Node.new
+        handler.expects(:details).with("fake").returns(nil)
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.environment'") do
+            result = handler.environment("fake")
+        end
+        assert_nil(result, "Got an environment for a node we could not find")
+
+        # Now for nodes we can find
+        handler = Node.new
+        node = SimpleNode.new("fake")
+        handler.expects(:details).with("fake").returns(node)
+        node.expects(:environment).returns("dev")
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.environment'") do
+            result = handler.environment("fake")
+        end
+        assert_equal("dev", result, "Did not get environment back")
     end
 
+    # Make sure our handler behaves rationally when it comes to getting parameter data.
     def test_parameters
-        raise "still failing"
+        # What happens when we can't find the node
+        handler = Node.new
+        handler.expects(:details).with("fake").returns(nil)
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.parameters'") do
+            result = handler.parameters("fake")
+        end
+        assert_nil(result, "Got parameters for a node we could not find")
+
+        # Now for nodes we can find
+        handler = Node.new
+        node = SimpleNode.new("fake")
+        handler.expects(:details).with("fake").returns(node)
+        node.expects(:parameters).returns({"a" => "b"})
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.parameters'") do
+            result = handler.parameters("fake")
+        end
+        assert_equal({"a" => "b"}, result, "Did not get parameters back")
     end
 
     def test_classes
-        raise "still failing"
+        # What happens when we can't find the node
+        handler = Node.new
+        handler.expects(:details).with("fake").returns(nil)
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.classes'") do
+            result = handler.classes("fake")
+        end
+        assert_nil(result, "Got classes for a node we could not find")
+
+        # Now for nodes we can find
+        handler = Node.new
+        node = SimpleNode.new("fake")
+        handler.expects(:details).with("fake").returns(node)
+        node.expects(:classes).returns(%w{yay foo})
+
+        result = nil
+        assert_nothing_raised("Could not call 'Node.classes'") do
+            result = handler.classes("fake")
+        end
+        assert_equal(%w{yay foo}, result, "Did not get classes back")
+    end
+end
+
+class TestSimpleNode < Test::Unit::TestCase
+    include NodeTesting
+
+    # Make sure we get all the defaults correctly.
+    def test_simplenode_initialize
+        node = nil
+        assert_nothing_raised("could not create a node without classes or parameters") do
+            node = SimpleNode.new("testing")
+        end
+        assert_equal("testing", node.name, "Did not set name correctly")
+        assert_equal({}, node.parameters, "Node parameters did not default correctly")
+        assert_equal([], node.classes, "Node classes did not default correctly")
+
+        # Now test it with values for both
+        params = {"a" => "b"}
+        classes = %w{one two}
+        assert_nothing_raised("could not create a node with classes and parameters") do
+            node = SimpleNode.new("testing", :parameters => params, :classes => classes)
+        end
+        assert_equal("testing", node.name, "Did not set name correctly")
+        assert_equal(params, node.parameters, "Node parameters did not get set correctly")
+        assert_equal(classes, node.classes, "Node classes did not get set correctly")
+
+        # And make sure a single class gets turned into an array
+        assert_nothing_raised("could not create a node with a class as a string") do
+            node = SimpleNode.new("testing", :classes => "test")
+        end
+        assert_equal(%w{test}, node.classes, "A node class string was not converted to an array")
+
+        # Make sure we get environments
+        assert_nothing_raised("could not create a node with an environment") do
+            node = SimpleNode.new("testing", :environment => "test")
+        end
+        assert_equal("test", node.environment, "Environment was not set")
+
+        # Now make sure we get the default env
+        Puppet[:environment] = "prod"
+        assert_nothing_raised("could not create a node with no environment") do
+            node = SimpleNode.new("testing")
+        end
+        assert_equal("prod", node.environment, "Did not get default environment")
+
+        # But that it stays nil if there's no default env set
+        Puppet[:environment] = ""
+        assert_nothing_raised("could not create a node with no environment and no default env") do
+            node = SimpleNode.new("testing")
+        end
+        assert_nil(node.environment, "Got a default env when none was set")
+
+    end
+
+    # Verify that the node source wins over facter.
+    def test_fact_merge
+        node = SimpleNode.new("yay", :parameters => {"a" => "one", "b" => "two"})
+
+        assert_nothing_raised("Could not merge parameters") do
+            node.fact_merge("b" => "three", "c" => "yay")
+        end
+        params = node.parameters
+        assert_equal("one", params["a"], "Lost nodesource parameters in parameter merge")
+        assert_equal("two", params["b"], "Overrode nodesource parameters in parameter merge")
+        assert_equal("yay", params["c"], "Did not get facts in parameter merge")
     end
 end
 
@@ -87,12 +398,16 @@ class TestNodeSources < Test::Unit::TestCase
 
         cleanup do
             Node.rm_node_source(:testing)
+            assert(! Node.const_defined?("Testing"), "Did not remove constant")
         end
     end
     
     def test_external_node_source
+        source = Node.node_source(:external)
+        assert(source, "Could not find external node source")
         mapper = mk_node_mapper
         searcher = mk_searcher(:external)
+        assert(searcher.fact_merge?, "External node source does not merge facts")
 
         # Make sure it gives the right response
         assert_equal({'classes' => %w{apple1 apple2 apple3}, :parameters => {"one" => "apple1", "two" => "apple2"}},
@@ -157,7 +472,10 @@ class TestNodeSources < Test::Unit::TestCase
     # This can stay in the main test suite because it doesn't actually use ldapsearch,
     # it just overrides the method so it behaves as though it were hitting ldap.
     def test_ldap_nodesearch
+        source = Node.node_source(:ldap)
+        assert(source, "Could not find ldap node source")
         searcher = mk_searcher(:ldap)
+        assert(searcher.fact_merge?, "LDAP node source does not merge facts")
 
         nodetable = {}