This is the first version where mongrel and webrick are reliably startable and stoppable via Puppet::Network::Server.

Added a network/server integration spec, testing startup, shutdown, reachability, and collision of webrick and mongrel servers in the new network code.

Converted Puppet::Network::HTTP::Handler class to a module, as mongrel Handler should be subclassed; converting subclasses to include the module instead.

Mongrel will actually stop if you .stop it, graceful_shutdown didn't seem quite so reliable.

Webrick requires running in its own Thread to avoid hanging the entire process; this requires introduction of a Mutex to make things safe.

We're only supporting the REST protocol.  Made this explicit.

Fixed http server setup args, w/ specs, ah the glory of integration testing.

6 files changed, 44 insertions, 19 deletions

diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index 773381c8d..7679bf320 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -1,4 +1,5 @@
-class Puppet::Network::HTTP::Handler
+module Puppet::Network::HTTP::Handler
+
     def initialize(args = {})
         raise ArgumentError unless @server = args[:server]
         raise ArgumentError unless @handler = args[:handler]
@@ -77,7 +78,7 @@ class Puppet::Network::HTTP::Handler
         %r{/#{@handler.to_s}s$}.match(path(request))
     end
     
-  # methods specific to a given web server
+  # methods to be overridden by the including web server class
     
     def register_handler
         raise NotImplementedError
diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb
index d948836cd..941ef0e43 100644
--- a/lib/puppet/network/http/mongrel.rb
+++ b/lib/puppet/network/http/mongrel.rb
@@ -13,20 +13,20 @@ class Puppet::Network::HTTP::Mongrel
         raise ArgumentError, ":address must be specified." unless args[:address]
         raise ArgumentError, ":port must be specified." unless args[:port]
         raise "Mongrel server is already listening" if listening?
-        
+
         @protocols = args[:protocols]
         @handlers = args[:handlers]
-        @server = Mongrel::HttpServer.new(args[:address], args[:port])
-
+        @server = Mongrel::HttpServer.new(args[:address], args[:port]) 
         setup_handlers
 
-        @server.run
         @listening = true
+        @server.run
     end
     
     def unlisten
         raise "Mongrel server is not listening" unless listening?
-        @server.graceful_shutdown
+        @server.stop
+        @server = nil
         @listening = false
     end
     
diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb
index 6c24e360c..7cb6f67bf 100644
--- a/lib/puppet/network/http/mongrel/rest.rb
+++ b/lib/puppet/network/http/mongrel/rest.rb
@@ -1,36 +1,45 @@
 require 'puppet/network/http/handler'
 
-class Puppet::Network::HTTP::MongrelREST < Puppet::Network::HTTP::Handler
+class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler
+
+  include Puppet::Network::HTTP::Handler
 
   private
-    
+ 
+    # have this mongrel @server listen for /foo and /foos REST endpoints
     def register_handler
         @server.register('/' + @handler.to_s, self)
         @server.register('/' + @handler.to_s + 's', self)
     end
     
+    # which HTTP verb was used in this request
     def http_method(request)
         request.params[Mongrel::Const::REQUEST_METHOD]
     end
     
+    # what path was requested?
     def path(request)
         # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com] 
         x = '/' + request.params[Mongrel::Const::REQUEST_PATH].split('/')[1]
     end
     
+    # return the key included in the request path
     def request_key(request)
         # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com] 
         x = request.params[Mongrel::Const::REQUEST_PATH].split('/')[2]        
     end
     
+    # return the request body
     def body(request)
         request.body
     end
     
+    # return the query params for this request
     def params(request)
         Mongrel::HttpRequest.query_parse(request.params["QUERY_STRING"])
     end
     
+    # produce the body of the response
     def encode_result(request, response, result, status = 200)
         response.start(status) do |head, body|
             body.write(result)
diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb
index 894e12473..3fd643612 100644
--- a/lib/puppet/network/http/webrick.rb
+++ b/lib/puppet/network/http/webrick.rb
@@ -1,10 +1,12 @@
 require 'webrick'
 require 'webrick/https'
 require 'puppet/network/http/webrick/rest'
+require 'thread'
 
 class Puppet::Network::HTTP::WEBrick
     def initialize(args = {})
         @listening = false
+        @mutex = Mutex.new
     end
     
     def listen(args = {})
@@ -12,24 +14,33 @@ class Puppet::Network::HTTP::WEBrick
         raise ArgumentError, ":protocols must be specified." if !args[:protocols] or args[:protocols].empty?
         raise ArgumentError, ":address must be specified." unless args[:address]
         raise ArgumentError, ":port must be specified." unless args[:port]
-        raise "WEBrick server is already listening" if listening?
         
         @protocols = args[:protocols]
         @handlers = args[:handlers]        
         @server = WEBrick::HTTPServer.new(:BindAddress => args[:address], :Port => args[:port])
         setup_handlers
-        @server.start
-        @listening = true
+
+        @mutex.synchronize do
+            raise "WEBrick server is already listening" if @listening        
+            @listening = true
+            @thread = Thread.new { @server.start }
+        end
     end
     
     def unlisten
-        raise "WEBrick server is not listening" unless listening?
-        @server.shutdown
-        @listening = false
+        @mutex.synchronize do
+            raise "WEBrick server is not listening" unless @listening
+            @server.shutdown
+            @thread.join
+            @server = nil
+            @listening = false
+        end
     end
     
     def listening?
-        @listening
+        @mutex.synchronize do
+            @listening
+        end
     end
     
   private
diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb
index 8cda079e2..923e002e3 100644
--- a/lib/puppet/network/http/webrick/rest.rb
+++ b/lib/puppet/network/http/webrick/rest.rb
@@ -1,6 +1,8 @@
 require 'puppet/network/http/handler'
 
-class Puppet::Network::HTTP::WEBrickREST < Puppet::Network::HTTP::Handler
+class Puppet::Network::HTTP::WEBrickREST
+
+    include Puppet::Network::HTTP::Handler
 
     # WEBrick uses a service() method to respond to requests.  Simply delegate to the handler response() method.
     def service(request, response)
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index 9e6b82469..cab14519b 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -1,3 +1,5 @@
+require 'puppet/network/http'
+
 class Puppet::Network::Server
 	attr_reader :server_type, :protocols, :address, :port
 
@@ -8,7 +10,7 @@ class Puppet::Network::Server
             raise(ArgumentError, "Must specify :address or configure Puppet :bindaddress.")
         @port = args[:port] || Puppet[:masterport] ||
             raise(ArgumentError, "Must specify :port or configure Puppet :masterport")
-        @protocols = []
+        @protocols = [ :rest ]
         @listening = false
         @routes = {}
         self.register(args[:handlers]) if args[:handlers]
@@ -38,8 +40,8 @@ class Puppet::Network::Server
   
     def listen
   	    raise "Cannot listen -- already listening." if listening?
-  	    http_server.listen(@routes.dup)
   	    @listening = true
+  	    http_server.listen(:address => address, :port => port, :handlers => @routes.keys, :protocols => protocols)
     end
   
     def unlisten