Merge branch 'routing' of http://git.rickbradley.com/puppet into routing

bin/puppetd

@@ -390,10 +390,10 @@ if Puppet[:listen] and ! options[:onetime]
     args[:Handlers] = handlers
     args[:Port] = Puppet[:puppetport]
 
-    require 'puppet/network/server/webrick'
+    require 'puppet/network/http_server/webrick'
 
     begin
-        server = Puppet::Network::Server::WEBrick.new(args)
+        server = Puppet::Network::HTTPServer::WEBrick.new(args)
     rescue => detail
         $stderr.puts detail
         puts detail.backtrace

bin/puppetmasterd

@@ -238,11 +238,11 @@ begin
     case Puppet[:servertype]
     when "webrick"
         # use the default, um, everything
-        require 'puppet/network/server/webrick'
-        webserver = server = Puppet::Network::Server::WEBrick.new(:Handlers => handlers)
+        require 'puppet/network/http_server/webrick'
+        webserver = server = Puppet::Network::HTTPServer::WEBrick.new(:Handlers => handlers)
     when "mongrel":
-        require 'puppet/network/server/mongrel'
-        server = Puppet::Network::Server::Mongrel.new(handlers)
+        require 'puppet/network/http_server/mongrel'
+        server = Puppet::Network::HTTPServer::Mongrel.new(handlers)
         addr = Puppet[:bindaddress]
         if addr == ""
             addr =  "127.0.0.1"

lib/puppet/defaults.rb

@@ -500,15 +500,6 @@ module Puppet
             "The server through which to send email reports."]
     )
 
-    # This needs to be in main because it's used too early in the system, such that
-    # we get an infinite loop otherwise.
-    self.setdefaults(:main,
-        :facts_terminus => ["yaml",
-            "The backend store to use for client facts."],
-        :checksum_terminus => ["file",
-            "The backend store to use for storing files by checksum (i.e., filebuckets)."]
-    )
-
     self.setdefaults(:rails,
         :dblocation => { :default => "$statedir/clientconfigs.sqlite3",
             :mode => 0660,

lib/puppet/feature/base.rb

@@ -18,7 +18,7 @@ Puppet.features.add(:libshadow, :libs => ["shadow"])
 Puppet.features.add(:root) { require 'puppet/util/suidmanager'; Puppet::Util::SUIDManager.uid == 0 }
 
 # We've got mongrel available
-Puppet.features.add(:mongrel, :libs => %w{rubygems mongrel puppet/network/server/mongrel})
+Puppet.features.add(:mongrel, :libs => %w{rubygems mongrel puppet/network/http_server/mongrel})
 
 # We have lcs diff
 Puppet.features.add :diff, :libs => %w{diff/lcs diff/lcs/hunk}

lib/puppet/indirector/indirection.rb

@@ -17,34 +17,26 @@ class Puppet::Indirector::Indirection
 
     # Create and return our cache terminus.
     def cache
-        terminus(cache_name)
+        raise(Puppet::DevError, "Tried to cache when no cache class was set") unless cache_class
+        terminus(cache_class)
     end
 
     # Should we use a cache?
     def cache?
-        cache_name ? true : false
+        cache_class ? true : false
     end
 
-    # Figure out the cache name, if there is one.  If there's no name, then
-    # caching is disabled.
-    def cache_name
-        unless @cache_name
-            setting_name = "%s_cache" % self.name
-            if ! Puppet.settings.valid?(setting_name) or (value = Puppet.settings[setting_name] and value == "none")
-                @cache_name = nil
-            else
-                @cache_name = value
-                @cache_name = @cache_name.intern if @cache_name.is_a?(String)
-            end
-        end
-        @cache_name
+    attr_reader :cache_class
+    # Define a terminus class to be used for caching.
+    def cache_class=(class_name)
+        validate_terminus_class(class_name)
+        @cache_class = class_name
     end
 
     # Clear our cached list of termini, and reset the cache name
     # so it's looked up again.
     # This is only used for testing.
     def clear_cache
-        @cache_name = nil
         @termini.clear
     end
 
@@ -65,7 +57,7 @@ class Puppet::Indirector::Indirection
         end
         @termini = {}
         @terminus_types = {}
-        @cache_name = nil
+        @cache_class = nil
         raise(ArgumentError, "Indirection %s is already defined" % @name) if @@indirections.find { |i| i.name == @name }
         @@indirections << self
     end
@@ -73,22 +65,31 @@ class Puppet::Indirector::Indirection
     # Return the singleton terminus for this indirection.
     def terminus(terminus_name = nil)
         # Get the name of the terminus.
-        unless terminus_name
-            param_name = "%s_terminus" % self.name
-            if Puppet.settings.valid?(param_name)
-                terminus_name = Puppet.settings[param_name]
-            else
-                terminus_name = Puppet[:default_terminus]
-            end
-            unless terminus_name and terminus_name.to_s != ""
-                raise ArgumentError, "Invalid terminus name %s" % terminus_name.inspect
-            end
-            terminus_name = terminus_name.intern if terminus_name.is_a?(String)
+        unless terminus_name ||= terminus_class
+            raise Puppet::DevError, "No terminus specified for %s; cannot redirect" % self.name
         end
         
         return @termini[terminus_name] ||= make_terminus(terminus_name)
     end
 
+    attr_reader :terminus_class
+
+    # Specify the terminus class to use.
+    def terminus_class=(terminus_class)
+        validate_terminus_class(terminus_class)
+        @terminus_class = terminus_class
+    end
+
+    # This is used by terminus_class= and cache=.
+    def validate_terminus_class(terminus_class)
+        unless terminus_class and terminus_class.to_s != ""
+            raise ArgumentError, "Invalid terminus name %s" % terminus_class.inspect
+        end
+        unless Puppet::Indirector::Terminus.terminus_class(terminus_class, self.name)
+            raise ArgumentError, "Could not find terminus %s for indirection %s" % [terminus_class, self.name]
+        end
+    end
+
     def find(*args)
         terminus.find(*args)
     end
@@ -111,10 +112,10 @@ class Puppet::Indirector::Indirection
     private
 
     # Create a new terminus instance.
-    def make_terminus(name)
+    def make_terminus(terminus_class)
         # Load our terminus class.
-        unless klass = Puppet::Indirector::Terminus.terminus_class(name, self.name)
-            raise ArgumentError, "Could not find terminus %s for indirection %s" % [name, self.name]
+        unless klass = Puppet::Indirector::Terminus.terminus_class(terminus_class, self.name)
+            raise ArgumentError, "Could not find terminus %s for indirection %s" % [terminus_class, self.name]
         end
         return klass.new
     end

lib/puppet/indirector/ldap/node.rb

@@ -21,8 +21,11 @@ class Puppet::Indirector::Ldap::Node < Puppet::Indirector::Ldap
                 raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
             end
             parents << parent
-            ldapsearch(parent) do |entry|
-                parent_info = process(parent, entry)
+
+            ldapsearch(parent) { |entry| parent_info = process(parent, entry) }
+
+            unless parent_info
+                raise Puppet::Error.new("Could not find parent node '%s'" % parent)
             end
             information[:classes] += parent_info[:classes]
             parent_info[:parameters].each do |param, value|

lib/puppet/network/http_server.rb

@@ -0,0 +1,3 @@
+# Just a stub, so we can correctly scope other classes.
+module Puppet::Network::HTTPServer # :nodoc:
+end

lib/puppet/network/http_server/mongrel.rb

@@ -31,7 +31,7 @@ require 'rubygems'
 require 'mongrel'
 require 'xmlrpc/server'
 require 'puppet/network/xmlrpc/server'
-require 'puppet/network/server'
+require 'puppet/network/http_server'
 require 'puppet/network/client_request'
 require 'puppet/daemon'
 
@@ -49,7 +49,7 @@ require 'resolv'
 # handler.xmlrpc_server.add_handler("my.add") { |a, b| a.to_i + b.to_i }
 # </pre>
 module Puppet::Network
-    class Server::Mongrel < ::Mongrel::HttpHandler
+    class HTTPServer::Mongrel < ::Mongrel::HttpHandler
         include Puppet::Daemon
         attr_reader :xmlrpc_server
 

lib/puppet/network/http_server/webrick.rb

@@ -6,7 +6,7 @@ require 'fcntl'
 
 require 'puppet/sslcertificates/support'
 require 'puppet/network/xmlrpc/webrick_servlet'
-require 'puppet/network/server'
+require 'puppet/network/http_server'
 require 'puppet/network/client'
 
 module Puppet
@@ -14,7 +14,7 @@ module Puppet
     module Network
         # The old-school, pure ruby webrick server, which is the default serving
         # mechanism.
-        class Server::WEBrick < WEBrick::HTTPServer
+        class HTTPServer::WEBrick < WEBrick::HTTPServer
             include Puppet::Daemon
             include Puppet::SSLCertificates::Support
 

lib/puppet/network/rest_server.rb

@@ -1,37 +0,0 @@
-class Puppet::Network::RESTServer
-  attr_reader :server
-  
-  def initialize(args = {})
-    raise(ArgumentError, "requires :server to be specified") unless args[:server]
-    @routes = {}
-    @listening = false
-    @server = args[:server]
-  end
-
-  def register(*indirections)
-    raise ArgumentError, "indirection names are required" if indirections.empty?
-    indirections.flatten.each { |i| @routes[i.to_sym] = true }
-  end
-  
-  def unregister(*indirections)
-    indirections = @routes.keys if indirections.empty?
-    indirections.flatten.each do |i|
-      raise(ArgumentError, "indirection [%s] is not known" % i) unless @routes[i.to_sym]
-      @routes.delete(i.to_sym)
-    end
-  end
-
-  def listening?
-    @listening
-  end
-  
-  def listen
-    raise "Cannot listen -- already listening" if listening?
-    @listening = true
-  end
-  
-  def unlisten
-    raise "Cannot unlisten -- not currently listening" unless listening?
-    @listening = false
-  end
-end

lib/puppet/network/server.rb

@@ -1,4 +1,66 @@
-# Just a stub, so we can correctly scope other classes.
-module Puppet::Network::Server # :nodoc:
+class Puppet::Network::Server
+  attr_reader :server_type
+
+  # which HTTP server subclass actually handles web requests of a certain type?  (e.g., :rest => RESTServer)
+  def self.server_class_by_name(name)
+    klass = (name.to_s + 'Server').to_sym
+    const_get klass
+  end
+  
+  # we will actually return an instance of the Server subclass which handles the HTTP web server, instead of 
+  # an instance of this generic Server class.  A tiny bit of sleight-of-hand is necessary to make this happen.
+  def self.new(args = {})
+    server_type = Puppet[:servertype] or raise "No servertype configuration found."
+    obj = self.server_class_by_name(server_type).allocate
+    obj.send :initialize, args.merge(:server_type => server_type)
+    obj
+  end
+  
+  def initialize(args = {})
+    @routes = {}
+    @listening = false
+    @server_type = args[:server_type]
+    self.register(args[:handlers]) if args[:handlers]
+  end
+
+  def register(*indirections)
+    raise ArgumentError, "indirection names are required" if indirections.empty?
+    indirections.flatten.each { |i| @routes[i.to_sym] = true }
+  end
+  
+  def unregister(*indirections)
+    indirections = @routes.keys if indirections.empty?
+    indirections.flatten.each do |i|
+      raise(ArgumentError, "indirection [%s] is not known" % i) unless @routes[i.to_sym]
+      @routes.delete(i.to_sym)
+    end
+  end
+
+  def listening?
+    @listening
+  end
+  
+  def listen
+    raise "Cannot listen -- already listening" if listening?
+    start_web_server
+    @listening = true
+  end
+  
+  def unlisten
+    raise "Cannot unlisten -- not currently listening" unless listening?
+    stop_web_server
+    @listening = false
+  end
+
+  private
+  
+  def start_web_server
+    raise NotImplementedError, "this method needs to be implemented by the actual web server (sub)class"
+  end
+  
+  def stop_web_server
+    raise NotImplementedError, "this method needs to be implemented by the actual web server (sub)class"
+  end
 end
 
+

lib/puppet/property.rb

@@ -56,8 +56,7 @@ class Property < Puppet::Parameter
 
     # Look up a value's name, so we can find options and such.
     def self.value_name(value)
-        name = symbolize(value)
-        if @parametervalues[name]
+        if value != '' and name = symbolize(value) and @parametervalues.include?(name)
             return name
         elsif ary = self.match?(value)
             return ary[0]

lib/puppet/provider/package/dpkg.rb

@@ -56,9 +56,18 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
         dpkg "-i", file
     end
 
+    def update
+        self.install
+    end
+
     # Return the version from the package.
     def latest
         output = dpkg_deb "--show", @resource[:source]
+        matches = /^(\S+)\t(\S+)$/.match(output).captures
+        unless matches[0].match(@resource[:name])
+            Puppet.warning "source doesn't contain named package, but %s" % matches[0]
+        end
+        matches[1]
     end
 
     def query

lib/puppet/provider/package/sun.rb

@@ -51,8 +51,6 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
                         unless names[name].nil?
                             hash[names[name]] = value
                         end
-                    else
-                        raise "Could not find %s" % name
                     end
                 when /\s+\d+.+/:
                     # nothing; we're ignoring the FILES info

lib/puppet/provider/service/freebsd.rb

@@ -0,0 +1,46 @@
+# Manage FreeBSD services.
+Puppet::Type.type(:service).provide :freebsd, :parent => :init do
+    desc "FreeBSD's (and probably NetBSD?) form of ``init``-style service
+        management; uses ``rc-update`` for service enabling and disabling."
+
+    commands :rcupdate => "/usr/local/sbin/rc-update"
+
+    defaultfor :operatingsystem => :freebsd
+
+    def self.defpath
+        superclass.defpath
+    end
+
+    def disable
+        begin
+            output = rcupdate("disable", @model[:name])
+        rescue Puppet::ExecutionFailure
+            raise Puppet::Error, "Could not disable %s: %s" %
+                [self.name, output]
+        end
+    end
+
+    def enabled?
+        begin
+            output = rcupdate("enabled", @model[:name])
+        rescue Puppet::ExecutionFailure
+            return :false
+        end
+
+        # If it's enabled, output is 0
+        if output =~ /^0$/
+            return :true
+        end
+
+        return :false
+    end
+
+    def enable
+        begin
+            output = rcupdate("enable", @model[:name])
+        rescue Puppet::ExecutionFailure => detail
+            raise Puppet::Error, "Could not enable %s: %s" %
+                [self.name, detail]
+        end
+    end
+end

spec/unit/indirector/indirection.rb

@@ -74,44 +74,43 @@ describe Puppet::Indirector::Indirection, " when managing indirection instances"
     end
 end
 
-describe Puppet::Indirector::Indirection, " when choosing terminus types" do
+describe Puppet::Indirector::Indirection, " when specifying terminus types" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
         @terminus = mock 'terminus'
         @terminus_class = stub 'terminus class', :new => @terminus
     end
 
-    it "should follow a convention on using per-model configuration parameters to determine the terminus class" do
-        Puppet.settings.expects(:valid?).with('test_terminus').returns(true)
-        Puppet.settings.expects(:value).with('test_terminus').returns(:foo)
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus.should equal(@terminus)
+    it "should allow specification of a terminus type" do
+        @indirection.should respond_to(:terminus_class=)
     end
 
-    it "should use a default system-wide configuration parameter parameter to determine the terminus class when no
-    per-model configuration parameter is available" do
-        Puppet.settings.expects(:valid?).with('test_terminus').returns(false)
-        Puppet.settings.expects(:value).with(:default_terminus).returns(:foo)
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus.should equal(@terminus)
-    end
-
-    it "should select the specified terminus class if a name is provided" do
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus(:foo).should equal(@terminus)
+    it "should fail to redirect if no terminus type has been specified" do
+        proc { @indirection.find("blah") }.should raise_error(Puppet::DevError)
     end
 
     it "should fail when the terminus class name is an empty string" do
-        proc { @indirection.terminus("") }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = "" }.should raise_error(ArgumentError)
     end
 
     it "should fail when the terminus class name is nil" do
-        proc { @indirection.terminus(nil) }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = nil }.should raise_error(ArgumentError)
     end
 
     it "should fail when the specified terminus class cannot be found" do
         Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(nil)
-        proc { @indirection.terminus(:foo) }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = :foo }.should raise_error(ArgumentError)
+    end
+
+    it "should select the specified terminus class if a terminus class name is provided" do
+        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus(:foo).should equal(@terminus)
+    end
+
+    it "should use the configured terminus class if no terminus name is specified" do
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus_class = :foo
+        @indirection.terminus().should equal(@terminus)
     end
 
     after do
@@ -163,30 +162,48 @@ describe Puppet::Indirector::Indirection, " when deciding whether to cache" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
         @terminus = mock 'terminus'
-        @indirection.stubs(:terminus).returns(@terminus)
+        @terminus_class = mock 'terminus class'
+        @terminus_class.stubs(:new).returns(@terminus)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus_class = :foo
+    end
+
+    it "should provide a method for setting the cache terminus class" do
+        @indirection.should respond_to(:cache_class=)
+    end
+
+    it "should fail to cache if no cache type has been specified" do
+        proc { @indirection.cache }.should raise_error(Puppet::DevError)
+    end
+
+    it "should fail to set the cache class when the cache class name is an empty string" do
+        proc { @indirection.cache_class = "" }.should raise_error(ArgumentError)
+    end
+
+    it "should fail to set the cache class when the cache class name is nil" do
+        proc { @indirection.cache_class = nil }.should raise_error(ArgumentError)
+    end
+
+    it "should fail to set the cache class when the specified cache class cannot be found" do
+        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(nil)
+        proc { @indirection.cache_class = :foo }.should raise_error(ArgumentError)
     end
 
     it "should not use a cache if there no cache setting" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(false)
         @indirection.expects(:cache).never
         @terminus.stubs(:save)
         @indirection.save(:whev)
     end
 
-    it "should not use a cache if the cache setting is set to 'none'" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(true)
-        Puppet.settings.expects(:value).with("test_cache").returns("none")
-        @indirection.expects(:cache).never
-        @terminus.stubs(:save)
-        @indirection.save(:whev)
-    end
-
-    it "should use a cache if there is a related cache setting and it is not set to 'none'" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(true)
-        Puppet.settings.expects(:value).with("test_cache").returns("something")
+    it "should use a cache if a cache was configured" do
         cache = mock 'cache'
         cache.expects(:save).with(:whev)
-        @indirection.expects(:cache).returns(cache)
+
+        cache_class = mock 'cache class'
+        cache_class.expects(:new).returns(cache)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:mycache, :test).returns(cache_class)
+
+        @indirection.cache_class = :mycache
         @terminus.stubs(:save)
         @indirection.save(:whev)
     end
@@ -199,7 +216,6 @@ end
 
 describe Puppet::Indirector::Indirection, " when using a cache" do
     before do
-        Puppet.settings.stubs(:valid?).returns(true)
         Puppet.settings.stubs(:value).with("test_terminus").returns("test_terminus")
         @terminus_class = mock 'terminus_class'
         @terminus = mock 'terminus'
@@ -209,11 +225,12 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
         Puppet::Indirector::Terminus.stubs(:terminus_class).with(:cache_terminus, :test).returns(@cache_class)
         Puppet::Indirector::Terminus.stubs(:terminus_class).with(:test_terminus, :test).returns(@terminus_class)
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
+        @indirection.terminus_class = :test_terminus
     end
 
     it "should copy all writing indirection calls to the cache terminus" do
         @cache_class.expects(:new).returns(@cache)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
+        @indirection.cache_class = :cache_terminus
         @cache.expects(:save).with(:whev)
         @terminus.stubs(:save)
         @indirection.save(:whev)
@@ -227,6 +244,7 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
     it "should reuse the cache terminus" do
         @cache_class.expects(:new).returns(@cache)
         Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
+        @indirection.cache_class = :cache_terminus
         @indirection.cache.should equal(@cache)
         @indirection.cache.should equal(@cache)
     end
@@ -234,19 +252,7 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
     it "should remove the cache terminus when all other terminus instances are cleared" do
         cache2 = mock 'cache2'
         @cache_class.stubs(:new).returns(@cache, cache2)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
-        @indirection.cache.should equal(@cache)
-        @indirection.clear_cache
-        @indirection.cache.should equal(cache2)
-    end
-
-    it "should look up the cache name when recreating the cache terminus after terminus instances have been cleared" do
-        cache_class2 = mock 'cache_class2'
-        cache2 = mock 'cache2'
-        cache_class2.expects(:new).returns(cache2)
-        @cache_class.expects(:new).returns(@cache)
-        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:other_cache, :test).returns(cache_class2)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus", "other_cache")
+        @indirection.cache_class = :cache_terminus
         @indirection.cache.should equal(@cache)
         @indirection.clear_cache
         @indirection.cache.should equal(cache2)

spec/unit/indirector/ldap/node.rb

@@ -82,7 +82,7 @@ describe Puppet::Indirector::Ldap::Node, " when searching for nodes" do
     end
 end
 
-describe Puppet::Indirector::Ldap::Node, " when a parent node exists" do
+describe Puppet::Indirector::Ldap::Node, " when a parent node is specified" do
     include LdapNodeSearching
 
     before do
@@ -99,16 +99,25 @@ describe Puppet::Indirector::Ldap::Node, " when a parent node exists" do
         @searcher.stubs(:parent_attribute).returns(:parent)
     end
 
+    it "should fail if the parent cannot be found" do
+        @connection.stubs(:search).with { |*args| args[2] == 'parent' }.returns("whatever")
+
+        @entry.stubs(:to_hash).returns({})
+        @entry.stubs(:vals).with(:parent).returns(%w{parent})
+
+        proc { @searcher.find("mynode") }.should raise_error(Puppet::Error)
+    end
+
     it "should add any parent classes to the node's classes" do
         @entry.stubs(:to_hash).returns({})
         @entry.stubs(:vals).with(:parent).returns(%w{parent})
-        @entry.stubs(:vals).with("one").returns(%w{a b})
+        @entry.stubs(:vals).with("classes").returns(%w{a b})
 
         @parent.stubs(:to_hash).returns({})
-        @parent.stubs(:vals).with("one").returns(%w{c d})
+        @parent.stubs(:vals).with("classes").returns(%w{c d})
         @parent.stubs(:vals).with(:parent).returns(nil)
 
-        @searcher.stubs(:class_attributes).returns(%w{one})
+        @searcher.stubs(:class_attributes).returns(%w{classes})
         @node.expects(:classes=).with(%w{a b c d})
         @searcher.find("mynode")
     end

spec/unit/network/rest_controller.rb

@@ -8,7 +8,58 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/network/rest_controller'
 
 describe Puppet::Network::RESTController, "in general" do
-  it "should take arguments from server, call the appropriate method with correct arguments (parameter passing)"
+  it "should route GET requests on indirector's name to indirector find for the model class"
+  it "should route GET requests on indirector's plural name to indirector search for the model class"
+  it "should route DELETE requests on indirector's name to indirector destroy for the model class"
+  it "should route POST requests on indirector's name to indirector save for the model class"  
   it "should serialize result data when methods are handled"
-  it "should serialize an error condition when indirection method call generates an exception"
+  it "should serialize an error condition when indirection method call generates an exception"  
+end
+
+__END__
+
+# possible implementation of the satisfying class
+
+class RESTController
+  def initialize(klass)
+    @klass = klass
+  end
+
+  # TODO: is it possible to distinguish from the request object the path which we were called by?
+  
+  def do_GET(request, response)
+    return do_GETS(request, response) if asked_for_plural?(request)
+    args = request.something
+    result = @klass.find args
+    return serialize(result)
+  end
+  
+  def do_GETS(request, response)
+    args = request.something
+    result = @klass.search args
+    return serialize(result)
+  end
+  
+  def do_DELETE(request, response)
+    args = request.something
+    result = @klass.destroy args
+    return serialize(result)
+  end
+  
+  def do_PUT(request, response)
+    args = request.something
+    obj = @klass.new(args)
+    result = obj.save
+    return serialize(result)
+  end
+  
+  def do_POST(request, response)
+    do_PUT(request, response)
+  end
+  
+  private
+  
+  def asked_for_plural?(request)
+    # TODO: pick apart the request and see if this was trying to do a plural or singular GET
+  end
 end

spec/unit/network/server.rb

@@ -5,17 +5,51 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/network/rest_server'
+require 'puppet/network/server'
 
-describe Puppet::Network::RESTServer, "when initializing" do
-  it "should require specifying which HTTP server will be used to provide access to clients" do
-    Proc.new { Puppet::Network::RESTServer.new }.should raise_error(ArgumentError)
+# a fake server class, so we don't have to implement full autoloading etc. (or at least just yet) just to do testing
+class TestServer < Puppet::Network::Server
+  def start_web_server
+  end
+  
+  def stop_web_server
   end
 end
 
-describe Puppet::Network::RESTServer, "in general" do
+describe Puppet::Network::Server, "when initializing" do
   before do
-    @server = Puppet::Network::RESTServer.new(:server => :mongrel)
+    Puppet::Network::Server.stubs(:server_class_by_name).returns(TestServer)
+  end
+  
+  it "should use the Puppet configurator to determine which HTTP server will be used to provide access to clients" do
+    Puppet.expects(:[]).with(:servertype).returns(:suparserver)
+    @server = Puppet::Network::Server.new
+    @server.server_type.should == :suparserver
+  end
+  
+  it "should fail to initialize if there is no HTTP server known to the Puppet configurator" do
+    Puppet.expects(:[]).with(:servertype).returns(nil)
+    Proc.new { Puppet::Network::Server.new }.should raise_error
+  end
+  
+  it "should use the Puppet Configurator to determine what style of service we will offer to clients (e.g., REST, XMLRPC, ...)"
+  it "should fail to initialize if there is no style of service known to the Puppet configurator"
+  
+  it "should allow registering indirections" do
+    @server = Puppet::Network::Server.new(:handlers => [ :foo, :bar, :baz])
+    Proc.new { @server.unregister(:foo, :bar, :baz) }.should_not raise_error
+  end
+  
+  it "should not be listening after initialization" do
+    Puppet::Network::Server.new.should_not be_listening
+  end
+end
+
+describe Puppet::Network::Server, "in general" do
+  before do
+    Puppet::Network::Server.stubs(:server_class_by_name).returns(TestServer)
+    Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
+    @server = Puppet::Network::Server.new
   end
   
   it "should allow registering an indirection for client access by specifying its indirection name" do
@@ -70,23 +104,27 @@ describe Puppet::Network::RESTServer, "in general" do
   end
   
   it "should provide a means of determining which HTTP server will be used to provide access to clients" do
-    @server.server.should == :mongrel
+    @server.server_type.should == :suparserver
   end
+  
+  it "should provide a means of determining which style of service is being offered to clients"
 
-  it "should allow for multiple configurations, each allowing a different HTTP server handling different indirections" do
-    @server2 = Puppet::Network::RESTServer.new(:server => :webrick)
+  it "should allow for multiple configurations, each handling different indirections" do
+    @server2 = Puppet::Network::Server.new
     @server.register(:foo, :bar)
     @server2.register(:foo, :xyzzy)
     @server.unregister(:foo, :bar)
     @server2.unregister(:foo, :xyzzy)
     Proc.new { @server.unregister(:xyzzy) }.should raise_error(ArgumentError)
-    Proc.new { @server2.unregister(:bar)}.should raise_error(ArgumentError)
+    Proc.new { @server2.unregister(:bar) }.should raise_error(ArgumentError)
   end  
 end
 
-describe Puppet::Network::RESTServer, "when listening is not turned on" do
+describe Puppet::Network::Server, "when listening is turned off" do
   before do
-    @server = Puppet::Network::RESTServer.new(:server => :mongrel)
+    Puppet::Network::Server.stubs(:server_class_by_name).returns(TestServer)
+    Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
+    @server = Puppet::Network::Server.new
   end
   
   it "should allow listening to be turned on" do
@@ -101,17 +139,23 @@ describe Puppet::Network::RESTServer, "when listening is not turned on" do
     @server.should_not be_listening
   end
   
-  it "should not route HTTP GET requests on indirector's name to indirector find for the specified HTTP server"
-  it "should not route HTTP GET requests on indirector's plural name to indirector search for the specified HTTP server"
-  it "should not route HTTP DELETE requests on indirector's name to indirector destroy for the specified HTTP server"
-  it "should not route HTTP POST requests on indirector's name to indirector save for the specified HTTP server"
+  it "should cause the HTTP server to listen when listening is turned on" do
+    @server.expects(:start_web_server)
+    @server.listen
+  end
+  
+  it "should not route HTTP GET requests to a controller for the registered indirection"
+  it "should not route HTTP DELETE requests to a controller for the registered indirection"
+  it "should not route HTTP POST requests to a controller for the registered indirection"
 
   # TODO: FIXME write integrations which fire up actual webrick / mongrel servers and are thus webrick / mongrel specific?]
 end
 
-describe Puppet::Network::RESTServer, "when listening is turned on" do
+describe Puppet::Network::Server, "when listening is turned on" do
   before do
-    @server = Puppet::Network::RESTServer.new(:server => :mongrel)
+    Puppet::Network::Server.stubs(:server_class_by_name).returns(TestServer)
+    Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
+    @server = Puppet::Network::Server.new
     @server.listen
   end
   
@@ -123,14 +167,18 @@ describe Puppet::Network::RESTServer, "when listening is turned on" do
     Proc.new { @server.listen }.should raise_error(RuntimeError)
   end
   
-  it "should indicate that it is listening" do
+  it "should indicate that listening is turned off" do
     @server.should be_listening
   end
+
+  it "should cause the HTTP server to stop listening when listening is turned off" do
+    @server.expects(:stop_web_server)
+    @server.unlisten
+  end
   
-  it "should route HTTP GET requests on indirector's name to indirector find for the specified HTTP server"
-  it "should route HTTP GET requests on indirector's plural name to indirector search for the specified HTTP server"
-  it "should route HTTP DELETE requests on indirector's name to indirector destroy for the specified HTTP server"
-  it "should route HTTP POST requests on indirector's name to indirector save for the specified HTTP server"
+  it "should route HTTP GET requests to a controller for the registered indirection"
+  it "should route HTTP DELETE requests to a controller for the registered indirection"
+  it "should route HTTP POST requests to a controller for the registered indirection"
 
   # TODO: FIXME [ write integrations which fire up actual webrick / mongrel servers and are thus webrick / mongrel specific?]
 end

test/lib/puppettest/servertest.rb

@@ -1,5 +1,5 @@
 require 'puppettest'
-require 'puppet/network/server/webrick'
+require 'puppet/network/http_server/webrick'
 
 module PuppetTest::ServerTest
     include PuppetTest
@@ -47,7 +47,7 @@ module PuppetTest::ServerTest
         # then create the actual server
         server = nil
         assert_nothing_raised {
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @@port,
                 :Handlers => handlers
             )

test/network/handler/ca.rb

@@ -191,7 +191,7 @@ class TestCA < Test::Unit::TestCase
         server = nil
         Puppet[:name] = "puppetmasterd"
         assert_nothing_raised {
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @@port,
                 :Handlers => {
                     :CA => {}, # so that certs autogenerate

test/network/server/mongrel_test.rb

@@ -12,7 +12,7 @@ class TestMongrelServer < PuppetTest::TestCase
 
     def mkserver(handlers = nil)
         handlers ||= { :Status => nil }
-        mongrel = Puppet::Network::Server::Mongrel.new(handlers)
+        mongrel = Puppet::Network::HTTPServer::Mongrel.new(handlers)
     end
 
     # Make sure client info is correctly extracted.

test/network/server/webrick.rb

@@ -3,7 +3,7 @@
 $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
 
 require 'puppettest'
-require 'puppet/network/server/webrick'
+require 'puppet/network/http_server/webrick'
 require 'mocha'
 
 class TestWebrickServer < Test::Unit::TestCase
@@ -19,7 +19,7 @@ class TestWebrickServer < Test::Unit::TestCase
     def test_basics
         server = nil
         assert_raise(Puppet::Error, "server succeeded with no cert") do
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @@port,
                 :Handlers => {
                     :Status => nil
@@ -28,7 +28,7 @@ class TestWebrickServer < Test::Unit::TestCase
         end
 
         assert_nothing_raised("Could not create simple server") do
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @@port,
                 :Handlers => {
                     :CA => {}, # so that certs autogenerate
@@ -112,7 +112,7 @@ class TestWebrickServer < Test::Unit::TestCase
     def mk_status_server
         server = nil
         assert_nothing_raised() {
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @@port,
                 :Handlers => {
                     :CA => {}, # so that certs autogenerate

test/ral/types/filesources.rb

@@ -548,7 +548,7 @@ class TestFileSources < Test::Unit::TestCase
 
         serverpid = nil
         assert_nothing_raised() {
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Handlers => {
                     :CA => {}, # so that certs autogenerate
                     :FileServer => {
@@ -593,7 +593,7 @@ class TestFileSources < Test::Unit::TestCase
 
         serverpid = nil
         assert_nothing_raised("Could not start on port %s" % @port) {
-            server = Puppet::Network::Server::WEBrick.new(
+            server = Puppet::Network::HTTPServer::WEBrick.new(
                 :Port => @port,
                 :Handlers => {
                     :CA => {}, # so that certs autogenerate

test/ral/types/property.rb

@@ -76,7 +76,8 @@ class TestProperty < Test::Unit::TestCase
             end
             assert_equal(/\d+/, name)
         end
-        ["two", :three].each do |value|
+        # these values should not have a name
+        ["two", :three, ''].each do |value|
             assert_nothing_raised do
                 name = property.value_name(value)
             end