Modified the 'master' handler to use the Catalog class to

compile node configurations, rather than using the Configuration
handler, which was never used directly.  I removed the Configuration
handler as a result.

Modified the 'master' handler (responsible for sending configurations
to clients) to always return Time.now as its compile date, so
configurations will always get recompiled.

9 files changed, 38 insertions, 474 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2e57e5f32..8dad0d977 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,12 @@
+    Modified the 'master' handler to use the Catalog class to
+    compile node configurations, rather than using the Configuration
+    handler, which was never used directly.  I removed the Configuration
+    handler as a result.
+
+    Modified the 'master' handler (responsible for sending configurations
+    to clients) to always return Time.now as its compile date, so
+    configurations will always get recompiled.
+
     Fixed #1184 -- definitions now autoload correctly all of the time.
 
     Removed the code from the client that tries to avoid recompiling
diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
index ecc340f75..2b5e8d912 100644
--- a/lib/puppet/indirector/catalog/compiler.rb
+++ b/lib/puppet/indirector/catalog/compiler.rb
@@ -19,7 +19,7 @@ class Puppet::Node::Catalog::Compiler < Puppet::Indirector::Code
         end
 
         if catalog = compile(node)
-            return catalog.to_transportable
+            return catalog
         else
             # This shouldn't actually happen; we should either return
             # a config or raise an exception.
@@ -44,22 +44,6 @@ class Puppet::Node::Catalog::Compiler < Puppet::Indirector::Code
         $0 =~ /puppetmasterd/
     end
 
-    # Return the catalog version.  Here we're returning the
-    # latest of the node, fact, or parse date.  These are the
-    # three things that go into compiling a client catalog,
-    # so changes in any of them result in changes.
-    # LAK:FIXME Note that this only works when all three sources
-    # use timestamps; once one of them moves to using real versions,
-    # the comparison stops working.
-    def version(key)
-        if node = Puppet::Node.find_by_any_name(key)
-            return [Puppet::Node.version(key).to_f, Puppet::Node::Facts.version(key).to_f, interpreter.catalog_version(node).to_f].sort[-1]
-        else
-            # This is the standard for "got nothing for ya".
-            0
-        end
-    end
-
     private
 
     # Add any extra data necessary to the node.
diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
deleted file mode 100644
index 8168ce1d5..000000000
--- a/lib/puppet/network/handler/configuration.rb
+++ /dev/null
@@ -1,184 +0,0 @@
-require 'openssl'
-require 'puppet'
-require 'puppet/parser/interpreter'
-require 'puppet/sslcertificates'
-require 'xmlrpc/server'
-require 'yaml'
-
-class Puppet::Network::Handler
-    class Configuration < Handler
-        desc "Puppet's configuration compilation interface.  Passed a node name
-            or other key, retrieves information about the node (using the ``node_source``)
-            and returns a compiled configuration."
-
-        include Puppet::Util
-
-        attr_accessor :local, :classes
-
-        @interface = XMLRPC::Service::Interface.new("configuration") { |iface|
-                iface.add_method("string configuration(string)")
-                iface.add_method("string version()")
-        }
-
-        # Compile a node's configuration.
-        def configuration(key, client = nil, clientip = nil)
-            # If we want to use the cert name as our key
-            if Puppet[:node_name] == 'cert' and client
-                key = client
-            end
-
-            # Note that this is reasonable, because either their node source should actually
-            # know about the node, or they should be using the ``none`` node source, which
-            # will always return data.
-            unless node = Puppet::Node.find_by_any_name(key)
-                raise Puppet::Error, "Could not find node '%s'" % key
-            end
-
-            # Add any external data to the node.
-            add_node_data(node)
-
-            configuration = compile(node)
-
-            return translate(configuration)
-        end
-
-        def initialize(options = {})
-            options.each do |param, value|
-                case param
-                when :Classes: @classes = value
-                when :Local: self.local = value
-                else
-                    raise ArgumentError, "Configuration handler does not accept %s" % param
-                end
-            end
-
-            set_server_facts
-        end
-
-        # Are we running locally, or are our clients networked?
-        def local?
-            self.local
-        end
-
-        # Return the configuration version.
-        def version(client = nil, clientip = nil)
-            if client and node = Puppet::Node.find_by_any_name(client)
-                update_node_check(node)
-                return interpreter.configuration_version(node)
-            else
-                # Just return something that will always result in a recompile, because
-                # this is local.
-                return (Time.now + 1000).to_i
-            end
-        end
-
-        private
-
-        # 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.merge(@server_facts)
-
-            # Add any specified classes to the node's class list.
-            if @classes
-                @classes.each do |klass|
-                    node.classes << klass
-                end
-            end
-        end
-
-        # Compile the actual configuration.
-        def compile(node)
-            # Pick the benchmark level.
-            if local?
-                level = :none
-            else
-                level = :notice
-            end
-
-            # Ask the interpreter to compile the configuration.
-            str = "Compiled configuration for %s" % node.name
-            if node.environment
-                str += " in environment %s" % node.environment
-            end
-            config = nil
-            benchmark(level, "Compiled configuration for %s" % node.name) do
-                begin
-                    config = interpreter.compile(node)
-                rescue => detail
-                    # If we're local, then we leave it to the local system
-                    # to handle error reporting, but otherwise we do it here
-                    # so the interpreter doesn't need to know if the parser
-                    # is local or not.
-                    Puppet.err(detail.to_s) unless local?
-                    raise
-                end
-            end
-
-            return config
-        end
-
-        # Create our interpreter object.
-        def create_interpreter
-            return Puppet::Parser::Interpreter.new
-        end
-
-        # Create/return our interpreter.
-        def interpreter
-            unless defined?(@interpreter) and @interpreter
-                @interpreter = create_interpreter
-            end
-            @interpreter
-        end
-
-        # Initialize our server fact hash; we add these to each client, and they
-        # won't change while we're running, so it's safe to cache the values.
-        def set_server_facts
-            @server_facts = {}
-
-            # Add our server version to the fact list
-            @server_facts["serverversion"] = Puppet.version.to_s
-
-            # And then add the server name and IP
-            {"servername" => "fqdn",
-                "serverip" => "ipaddress"
-            }.each do |var, fact|
-                if value = Facter.value(fact)
-                    @server_facts[var] = value
-                else
-                    Puppet.warning "Could not retrieve fact %s" % fact
-                end
-            end
-
-            if @server_facts["servername"].nil?
-                host = Facter.value(:hostname)
-                if domain = Facter.value(:domain)
-                    @server_facts["servername"] = [host, domain].join(".")
-                else
-                    @server_facts["servername"] = host
-                end
-            end
-        end
-
-        # Translate our configuration appropriately for sending back to a client.
-        def translate(config)
-            if local?
-                config
-            else
-                CGI.escape(config.to_yaml(:UseBlock => true))
-            end
-        end
-
-        # Mark that the node has checked in. FIXME this needs to be moved into
-        # the Node class, or somewhere that's got abstract backends.
-        def update_node_check(node)
-            if Puppet.features.rails? and Puppet[:storeconfigs]
-                Puppet::Rails.connect
-
-                host = Puppet::Rails::Host.find_or_create_by_name(node.name)
-                host.last_freshcheck = Time.now
-                host.save
-            end
-        end
-    end
-end
diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
index dabfaca50..851ccc7b2 100644
--- a/lib/puppet/network/handler/master.rb
+++ b/lib/puppet/network/handler/master.rb
@@ -23,8 +23,8 @@ class Puppet::Network::Handler
 
         # Tell a client whether there's a fresh config for it
         def freshness(client = nil, clientip = nil)
-            client ||= Facter.value("hostname")
-            config_handler.version(client, clientip)
+            # Always force a recompile.  Newer clients shouldn't do this (as of April 2008).
+            Time.now
         end
 
         def initialize(hash = {})
@@ -51,8 +51,6 @@ class Puppet::Network::Handler
             if hash.include?(:Classes)
                 args[:Classes] = hash[:Classes]
             end
-
-            @config_handler = Puppet::Network::Handler.handler(:configuration).new(args)
         end
 
         # Call our various handlers; this handler is getting deprecated.
@@ -63,13 +61,9 @@ class Puppet::Network::Handler
             # Pass the facts to the fact handler
             Puppet::Node::Facts.new(client, facts).save unless local?
 
-            # And get the configuration from the config handler
-            config = nil
-            benchmark(:notice, "Compiled configuration for %s" % client) do
-                config = config_handler.configuration(client)
-            end
+            catalog = Puppet::Node::Catalog.find(client)
 
-            return translate(config.extract)
+            return translate(catalog.extract)
         end
 
         private
@@ -93,13 +87,6 @@ class Puppet::Network::Handler
             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
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 4b48ff6cf..d214a60ee 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -303,6 +303,12 @@ class Puppet::Parser::Resource
         return bucket
     end
 
+    # Convert this resource to a RAL resource.  We hackishly go via the
+    # transportable stuff.
+    def to_type
+        to_trans.to_type
+    end
+
     def to_transobject
         # Now convert to a transobject
         obj = Puppet::TransObject.new(@ref.title, @ref.type)
diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb
index 57bbcdbd0..083a9ced5 100755
--- a/spec/unit/indirector/catalog/compiler.rb
+++ b/spec/unit/indirector/catalog/compiler.rb
@@ -145,10 +145,10 @@ describe Puppet::Node::Catalog::Compiler, " when creating catalogs" do
 
     it "should return the results of compiling as the catalog" do
         config = mock 'config'
-        result = mock 'result', :to_transportable => :catalog
+        result = mock 'result'
 
         @compiler.interpreter.expects(:compile).with(@node).returns(result)
-        @compiler.find(@request).should == :catalog
+        @compiler.find(@request).should equal(result)
     end
 
     it "should benchmark the compile process" do
@@ -160,53 +160,3 @@ describe Puppet::Node::Catalog::Compiler, " when creating catalogs" do
         @compiler.find(@request)
     end
 end
-
-describe Puppet::Node::Catalog::Compiler, " when determining a client's available catalog version" do
-    before do
-        Puppet::Node::Facts.stubs(:find).returns(nil)
-        Facter.stubs(:value).returns("whatever")
-        @catalog = Puppet::Node::Catalog::Compiler.new
-        @name = "johnny"
-    end
-
-    it "should provide a mechanism for providing the version of a given client's catalog" do
-        @catalog.should respond_to(:version)
-    end
-
-    it "should use the client's Facts version as the available catalog version if it is the most recent" do
-        Puppet::Node::Facts.stubs(:version).with(@name).returns(5)
-        Puppet::Node.expects(:version).with(@name).returns(3)
-        @catalog.interpreter.stubs(:catalog_version).returns(4)
-
-        @catalog.version(@name).should == 5
-    end
-
-    it "should use the client's Node version as the available catalog version if it is the most recent" do
-        Puppet::Node::Facts.stubs(:version).with(@name).returns(3)
-        Puppet::Node.expects(:version).with(@name).returns(5)
-        @catalog.interpreter.stubs(:catalog_version).returns(4)
-
-        @catalog.version(@name).should == 5
-    end
-
-    it "should use the last parse date as the available catalog version if it is the most recent" do
-        Puppet::Node::Facts.stubs(:version).with(@name).returns(3)
-        Puppet::Node.expects(:version).with(@name).returns(4)
-        @catalog.interpreter.stubs(:catalog_version).returns(5)
-
-        @catalog.version(@name).should == 5
-    end
-
-    it "should return a version of 0 if no information on the node can be found" do
-        Puppet::Node.stubs(:find_by_any_name).returns(nil)
-        @catalog.version(@name).should == 0
-    end
-
-    it "should indicate when an update is available even if an input has clock skew" do
-        pending "Unclear how to implement this"
-    end
-
-    it "should not indicate an available update when apparent updates are a result of clock skew" do
-        pending "Unclear how to implement this"
-    end
-end
diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb
index 776e9c742..6b2021916 100755
--- a/spec/unit/parser/resource.rb
+++ b/spec/unit/parser/resource.rb
@@ -64,6 +64,13 @@ describe Puppet::Parser::Resource do
         @resource[:one].should == "yay"
     end
 
+    it "should have a method for converting to a ral resource" do
+        trans = mock 'trans', :to_type => "yay"
+        @resource = mkresource
+        @resource.expects(:to_trans).returns trans
+        @resource.to_type.should == "yay"
+    end
+
     describe "when initializing" do
         before do
             @arguments = {:type => "resource", :title => "testing", :scope => stub('scope', :source => mock('source'))}
diff --git a/test/network/handler/configuration.rb b/test/network/handler/configuration.rb
deleted file mode 100755
index 36c5d9e54..000000000
--- a/test/network/handler/configuration.rb
+++ /dev/null
@@ -1,160 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../lib/puppettest'
-
-require 'puppettest'
-require 'puppet/network/handler/configuration'
-
-class TestHandlerConfiguration < Test::Unit::TestCase
-	include PuppetTest
-
-    Config = Puppet::Network::Handler.handler(:configuration)
-
-    # Check all of the setup stuff.
-    def test_initialize
-        config = nil
-        assert_nothing_raised("Could not create local config") do
-            config = Config.new(:Local => true)
-        end
-
-        assert(config.local?, "Config is not considered local after being started that way")
-    end
-
-    # Test creation/returning of the interpreter
-    def test_interpreter
-        config = Config.new
-
-        # First test the defaults
-        config.expects(:create_interpreter).returns(:interp)
-        assert_equal(:interp, config.send(:interpreter), "Did not return the interpreter")
-
-        # Now run it again and make sure we get the same thing
-        assert_equal(:interp, config.send(:interpreter), "Did not cache the interpreter")
-    end
-
-    def test_create_interpreter
-        config = Config.new(:Local => false)
-        args = {}
-
-        # Try it first with defaults.
-        Puppet::Parser::Interpreter.expects(:new).returns(:interp)
-        assert_equal(:interp, config.send(:create_interpreter), "Did not return the interpreter")
-    end
-
-    # Make sure node objects get appropriate data added to them.
-    def test_add_node_data
-        # First with no classes
-        config = Config.new
-
-        fakenode = Object.new
-        # Set the server facts to something
-        config.instance_variable_set("@server_facts", :facts)
-        fakenode.expects(:merge).with(:facts)
-        config.send(:add_node_data, fakenode)
-
-        # Now try it with classes.
-        config.classes = %w{a b}
-        list = []
-        fakenode = Object.new
-        fakenode.expects(:merge).with(:facts)
-        fakenode.expects(:classes).returns(list).times(2)
-        config.send(:add_node_data, fakenode)
-        assert_equal(%w{a b}, list, "Did not add classes to node")
-    end
-
-    def test_compile
-        config = Config.new
-
-        # First do a local
-        node = mock 'node'
-        node.stubs(:name).returns(:mynode)
-        node.stubs(:environment).returns(:myenv)
-
-        interp = mock 'interpreter'
-        interp.stubs(:environment)
-        interp.expects(:compile).with(node).returns(:config)
-        config.expects(:interpreter).returns(interp)
-
-        Puppet.expects(:notice) # The log message from benchmarking
-
-        assert_equal(:config, config.send(:compile, node), "Did not return config")
-        
-        # Now try it non-local
-        node = mock 'node'
-        node.stubs(:name).returns(:mynode)
-        node.stubs(:environment).returns(:myenv)
-
-        interp = mock 'interpreter'
-        interp.stubs(:environment)
-        interp.expects(:compile).with(node).returns(:config)
-
-        config = Config.new(:Local => true)
-        config.expects(:interpreter).returns(interp)
-
-        assert_equal(:config, config.send(:compile, node), "Did not return config")
-    end
-
-    def test_set_server_facts
-        config = Config.new
-        assert_nothing_raised("Could not call :set_server_facts") do
-            config.send(:set_server_facts)
-        end
-        facts = config.instance_variable_get("@server_facts")
-        %w{servername serverversion serverip}.each do |fact|
-            assert(facts.include?(fact), "Config did not set %s fact" % fact)
-        end
-    end
-
-    def test_translate
-        # First do a local config
-        config = Config.new(:Local => true)
-        assert_equal(:plain, config.send(:translate, :plain), "Attempted to translate local config")
-
-        # Now a non-local
-        config = Config.new(:Local => false)
-        assert(! config.local?, "Config wrongly thinks it's local")
-        obj = mock 'dumpee'
-        yamld = mock 'yaml'
-        obj.expects(:to_yaml).with(:UseBlock => true).returns(yamld)
-        CGI.expects(:escape).with(yamld).returns(:translated)
-        assert_equal(:translated, config.send(:translate, obj), "Did not return translated config")
-    end
-
-    # Check that we're storing the node freshness into the rails db.  Hackilicious.
-    def test_update_node_check
-        # This is stupid.
-        config = Config.new
-        node = Object.new
-        node.expects(:name).returns(:hostname)
-        now = Object.new
-        Time.expects(:now).returns(now)
-        host = Object.new
-        host.expects(:last_freshcheck=).with(now)
-        host.expects(:save)
-
-        # Only test the case where rails is there
-        Puppet[:storeconfigs] = true
-        Puppet.features.expects(:rails?).returns(true)
-        Puppet::Rails.expects(:connect)
-        Puppet::Rails::Host.expects(:find_or_create_by_name).with(:hostname).returns(host)
-
-        config.send(:update_node_check, node)
-    end
-
-    def test_version
-        # First try the case where we can't look up the node
-        config = Config.new
-        node = Object.new
-        Puppet::Node.stubs(:find_by_any_name).with(:client).returns(false, node)
-        interp = Object.new
-        assert_instance_of(Bignum, config.version(:client), "Did not return configuration version")
-
-        # And then when we find the node.
-        config = Config.new
-        config.expects(:update_node_check).with(node)
-        interp = Object.new
-        interp.expects(:configuration_version).returns(:version)
-        config.expects(:interpreter).returns(interp)
-        assert_equal(:version, config.version(:client), "Did not return configuration version")
-    end
-end
diff --git a/test/network/handler/master.rb b/test/network/handler/master.rb
index 55522237b..e91ec2f47 100755
--- a/test/network/handler/master.rb
+++ b/test/network/handler/master.rb
@@ -13,52 +13,17 @@ class TestMaster < Test::Unit::TestCase
         Puppet::Indirector::Indirection.clear_cache
     end
 
-    # Make sure that files are reread when they change.
-    def test_filereread
-        # Start with a normal setting
-        Puppet[:filetimeout] = 15
-
-        manifest = mktestmanifest()
-
-        facts = Puppet::Network::Client.master.facts
-        # Store them, so we don't determine frshness based on facts.
-        Puppet::Util::Storage.cache(:configuration)[:facts] = facts
-
-        file2 = @createdfile + "2"
-        @@tmpfiles << file2
-
-        client = master = nil
-        Puppet[:manifest] = manifest
-        assert_nothing_raised() {
-            # this is the default server setup
-            master = Puppet::Network::Handler.master.new(
-                :Local => true
-            )
-        }
-
-        config = master.getconfig({"hostname" => "blah"})
-
-        # Cache this value for later
-        parse1 = master.freshness("mynode")
-
-        sleep 1.5
-        # Create a new manifest
-        File.open(manifest, "w") { |f|
-            f.puts "file { \"%s\": ensure => file }\n" % file2
-        }
-
-        # Verify that the master doesn't immediately reparse the file; we
-        # want to wait through the timeout
-        assert_equal(parse1, master.freshness("mynode"), "Master did not wait through timeout")
-
-        # Then eliminate it
-        Puppet[:filetimeout] = 0
+    def test_freshness_is_always_now
+        master = Puppet::Network::Handler.master.new(
+            :Manifest => tempfile,
+            :UseNodes => true,
+            :Local => true
+        )
 
-        # Now make sure the master does reparse
-        #Puppet.notice "%s vs %s" % [parse1, master.freshness]
-        assert(parse1 != master.freshness("mynode"), "Master did not reparse file")
+        now1 = mock 'now1'
+        Time.expects(:now).returns(now1)
 
-        assert(master.getconfig({"hostname" => "blah"}) != config, "Did not use reloaded config")
+        assert_equal(master.freshness, now1, "Did not return current time as freshness")
     end
 
     # Make sure we're correctly doing clientname manipulations.