Adding configuration support for XMLRPC handlers.

This provides the interface for configuring backward compatibility
in the new-style server code.  It requires a completely different
configuration step, in that REST and xmlrpc handlers are configured
separately, rather than using the same handler names, but this was
considered less evil than having a large abstraction layer for specifying
how the handlers are related.

2 files changed, 255 insertions, 125 deletions

diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index fb78baf8e..a610ea7c9 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -6,21 +6,28 @@ class Puppet::Network::Server
     def initialize(args = {})
         @server_type = Puppet[:servertype] or raise "No servertype configuration found."  # e.g.,  WEBrick, Mongrel, etc.
         http_server_class || raise(ArgumentError, "Could not determine HTTP Server class for server type [#{@server_type}]")
-        @address = args[:address] || Puppet[:bindaddress] || 
-            raise(ArgumentError, "Must specify :address or configure Puppet :bindaddress.")
-        @port = args[:port] || Puppet[:masterport] ||
-            raise(ArgumentError, "Must specify :port or configure Puppet :masterport")
-        @protocols = [ :rest ]
+
+        @address = args[:address] || Puppet[:bindaddress] || raise(ArgumentError, "Must specify :address or configure Puppet :bindaddress.")
+        @port = args[:port] || Puppet[:masterport] || raise(ArgumentError, "Must specify :port or configure Puppet :masterport")
+
+        @protocols = [ :rest, :xmlrpc ]
         @listening = false
         @routes = {}
+        @xmlrpc_routes = {}
         self.register(args[:handlers]) if args[:handlers]
+        self.register_xmlrpc(args[:xmlrpc_handlers]) if args[:xmlrpc_handlers]
     end
 
+    # Register handlers for REST networking, based on the Indirector.
     def register(*indirections)
         raise ArgumentError, "Indirection names are required." if indirections.empty?
-        indirections.flatten.each { |i| @routes[i.to_sym] = true }
+        indirections.flatten.each do |name|
+            Puppet::Indirector::Indirection.model(name) || raise(ArgumentError, "Cannot locate indirection '#{name}'.")
+            @routes[name.to_sym] = true
+        end
     end
   
+    # Unregister Indirector handlers.
     def unregister(*indirections)
         raise "Cannot unregister indirections while server is listening." if listening?
         indirections = @routes.keys if indirections.empty?
@@ -34,6 +41,29 @@ class Puppet::Network::Server
         end
     end
 
+    # Register xmlrpc handlers for backward compatibility.
+    def register_xmlrpc(*namespaces)
+        raise ArgumentError, "XMLRPC namespaces are required." if namespaces.empty?
+        namespaces.flatten.each do |name|
+            Puppet::Network::Handler.handler(name) || raise(ArgumentError, "Cannot locate XMLRPC handler for namespace '#{name}'.")
+            @xmlrpc_routes[name.to_sym] = true
+        end
+    end
+  
+    # Unregister xmlrpc handlers.
+    def unregister_xmlrpc(*namespaces)
+        raise "Cannot unregister xmlrpc handlers while server is listening." if listening?
+        namespaces = @xmlrpc_routes.keys if namespaces.empty?
+
+        namespaces.flatten.each do |i|
+            raise(ArgumentError, "XMLRPC handler '%s' is unknown." % i) unless @xmlrpc_routes[i.to_sym]
+        end
+        
+        namespaces.flatten.each do |i|
+            @xmlrpc_routes.delete(i.to_sym)
+        end
+    end
+
     def listening?
         @listening
     end
@@ -41,7 +71,7 @@ class Puppet::Network::Server
     def listen
         raise "Cannot listen -- already listening." if listening?
         @listening = true
-        http_server.listen(:address => address, :port => port, :handlers => @routes.keys, :protocols => protocols)
+        http_server.listen(:address => address, :port => port, :handlers => @routes.keys, :xmlrpc_handlers => @xmlrpc_routes.keys, :protocols => protocols)
     end
   
     def unlisten
diff --git a/spec/unit/network/server.rb b/spec/unit/network/server.rb
index ef9e85a73..5c1c63520 100644
--- a/spec/unit/network/server.rb
+++ b/spec/unit/network/server.rb
@@ -11,8 +11,11 @@ describe Puppet::Network::Server, "when initializing" do
         @mock_http_server_class = mock('http server class')
         Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
         Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class)
+
+        Puppet::Indirector::Indirection.stubs(:model).returns mock('indirection')
+        Puppet::Network::Handler.stubs(:handler).returns mock('xmlrpc_handler')
     end
-    
+
     it "should allow specifying a listening address" do
         Puppet.stubs(:[]).with(:masterport).returns('')
         @server = Puppet::Network::Server.new(:address => "127.0.0.1")
@@ -24,7 +27,7 @@ describe Puppet::Network::Server, "when initializing" do
         @server = Puppet::Network::Server.new(:port => 31337)
         @server.port.should == 31337
     end
-    
+
     it "should use the Puppet configurator to find a default listening address" do
         Puppet.stubs(:[]).with(:masterport).returns('')
         Puppet.expects(:[]).with(:bindaddress).returns("10.0.0.1")
@@ -42,13 +45,13 @@ describe Puppet::Network::Server, "when initializing" do
     it "should fail to initialize if no listening address can be found" do
         Puppet.stubs(:[]).with(:masterport).returns(6667)
         Puppet.stubs(:[]).with(:bindaddress).returns(nil)
-        Proc.new { Puppet::Network::Server.new }.should raise_error(ArgumentError)
+        lambda { Puppet::Network::Server.new }.should raise_error(ArgumentError)
     end
-    
+
     it "should fail to initialize if no listening port can be found" do
         Puppet.stubs(:[]).with(:bindaddress).returns("127.0.0.1")
         Puppet.stubs(:[]).with(:masterport).returns(nil)
-        Proc.new { Puppet::Network::Server.new }.should raise_error(ArgumentError)        
+        lambda { Puppet::Network::Server.new }.should raise_error(ArgumentError)        
     end
 
     it "should use the Puppet configurator to determine which HTTP server will be used to provide access to clients" do
@@ -56,126 +59,208 @@ describe Puppet::Network::Server, "when initializing" do
         @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337)
         @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(:address => "127.0.0.1", :port => 31337) }.should raise_error
+        lambda { Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337) }.should raise_error
     end
- 
+
     it "should ask the Puppet::Network::HTTP class to fetch the proper HTTP server class" do
         Puppet::Network::HTTP.expects(:server_class_by_type).with(:suparserver).returns(@mock_http_server_class)
         @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337)
     end
-    
+
     it "should fail if the HTTP server class is unknown" do
         Puppet::Network::HTTP.stubs(:server_class_by_type).returns(nil)
-        Proc.new { Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337) }.should raise_error(ArgumentError)
+        lambda { Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337) }.should raise_error(ArgumentError)
     end
-  
-    it "should allow registering indirections" do
+
+    it "should allow registering REST handlers" do
         @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337, :handlers => [ :foo, :bar, :baz])
-        Proc.new { @server.unregister(:foo, :bar, :baz) }.should_not raise_error
+        lambda { @server.unregister(:foo, :bar, :baz) }.should_not raise_error
     end
-  
+
+    it "should allow registering XMLRPC handlers" do
+        @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337, :xmlrpc_handlers => [ :foo, :bar, :baz])
+        lambda { @server.unregister_xmlrpc(:foo, :bar, :baz) }.should_not raise_error
+    end
+
     it "should not be listening after initialization" do
         Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337).should_not be_listening
     end
 end
 
-describe Puppet::Network::Server, "in general" do
+describe Puppet::Network::Server do
     before do
         @mock_http_server_class = mock('http server class')
         Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class)
         Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
         @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337)
     end
-  
-    it "should allow registering an indirection for client access by specifying its indirection name" do
-        Proc.new { @server.register(:foo) }.should_not raise_error
-    end
-  
-    it "should require at least one indirection name when registering indirections for client access" do
-        Proc.new { @server.register }.should raise_error(ArgumentError)
-    end
-  
-    it "should allow for numerous indirections to be registered at once for client access" do
-        Proc.new { @server.register(:foo, :bar, :baz) }.should_not raise_error
-    end
 
-    it "should allow the use of indirection names to specify which indirections are to be no longer accessible to clients" do
-        @server.register(:foo)
-        Proc.new { @server.unregister(:foo) }.should_not raise_error    
-    end
+    describe "when managing indirection registrations" do
+        before do
+            Puppet::Indirector::Indirection.stubs(:model).returns mock('indirection')
+        end
 
-    it "should leave other indirections accessible to clients when turning off indirections" do
-        @server.register(:foo, :bar)
-        @server.unregister(:foo)
-        Proc.new { @server.unregister(:bar)}.should_not raise_error
-    end
-  
-    it "should allow specifying numerous indirections which are to be no longer accessible to clients" do
-        @server.register(:foo, :bar)
-        Proc.new { @server.unregister(:foo, :bar) }.should_not raise_error
-    end
-    
-    it "should not turn off any indirections if given unknown indirection names to turn off" do
-        @server.register(:foo, :bar)
-        Proc.new { @server.unregister(:foo, :bar, :baz) }.should raise_error(ArgumentError)
-        Proc.new { @server.unregister(:foo, :bar) }.should_not raise_error
-    end
-  
-    it "should not allow turning off unknown indirection names" do
-        @server.register(:foo, :bar)
-        Proc.new { @server.unregister(:baz) }.should raise_error(ArgumentError)
-    end
-  
-    it "should disable client access immediately when turning off indirections" do
-        @server.register(:foo, :bar)
-        @server.unregister(:foo)    
-        Proc.new { @server.unregister(:foo) }.should raise_error(ArgumentError)
-    end
-  
-    it "should allow turning off all indirections at once" do
-        @server.register(:foo, :bar)
-        @server.unregister
-        [ :foo, :bar, :baz].each do |indirection|
-            Proc.new { @server.unregister(indirection) }.should raise_error(ArgumentError)
+        it "should allow registering an indirection for client access by specifying its indirection name" do
+            lambda { @server.register(:foo) }.should_not raise_error
+        end
+
+        it "should require that the indirection be valid" do
+            Puppet::Indirector::Indirection.expects(:model).with(:foo).returns nil
+            lambda { @server.register(:foo) }.should raise_error(ArgumentError)
+        end
+
+        it "should require at least one indirection name when registering indirections for client access" do
+            lambda { @server.register }.should raise_error(ArgumentError)
+        end
+
+        it "should allow for numerous indirections to be registered at once for client access" do
+            lambda { @server.register(:foo, :bar, :baz) }.should_not raise_error
+        end
+
+        it "should allow the use of indirection names to specify which indirections are to be no longer accessible to clients" do
+            @server.register(:foo)
+            lambda { @server.unregister(:foo) }.should_not raise_error    
+        end
+
+        it "should leave other indirections accessible to clients when turning off indirections" do
+            @server.register(:foo, :bar)
+            @server.unregister(:foo)
+            lambda { @server.unregister(:bar)}.should_not raise_error
+        end
+
+        it "should allow specifying numerous indirections which are to be no longer accessible to clients" do
+            @server.register(:foo, :bar)
+            lambda { @server.unregister(:foo, :bar) }.should_not raise_error
+        end
+
+        it "should not turn off any indirections if given unknown indirection names to turn off" do
+            @server.register(:foo, :bar)
+            lambda { @server.unregister(:foo, :bar, :baz) }.should raise_error(ArgumentError)
+            lambda { @server.unregister(:foo, :bar) }.should_not raise_error
+        end
+
+        it "should not allow turning off unknown indirection names" do
+            @server.register(:foo, :bar)
+            lambda { @server.unregister(:baz) }.should raise_error(ArgumentError)
+        end
+
+        it "should disable client access immediately when turning off indirections" do
+            @server.register(:foo, :bar)
+            @server.unregister(:foo)    
+            lambda { @server.unregister(:foo) }.should raise_error(ArgumentError)
+        end
+
+        it "should allow turning off all indirections at once" do
+            @server.register(:foo, :bar)
+            @server.unregister
+            [ :foo, :bar, :baz].each do |indirection|
+                lambda { @server.unregister(indirection) }.should raise_error(ArgumentError)
+            end
         end
     end
-  
+
     it "should provide a means of determining whether it is listening" do
         @server.should respond_to(:listening?)
     end
-  
+
     it "should provide a means of determining which HTTP server will be used to provide access to clients" do
         @server.server_type.should == :suparserver
     end
-    
-    it "should allow for multiple configurations, each handling different indirections" do
-        @server2 = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337)
-        @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)
-    end  
 
     it "should provide a means of determining which protocols are in use" do
         @server.should respond_to(:protocols)
     end
-    
-    it "should only support the REST protocol at this time" do
-        @server.protocols.should == [ :rest ]
+
+    it "should set the protocols to :rest and :xmlrpc" do
+        @server.protocols.should == [ :rest, :xmlrpc ]
     end
-    
+
     it "should provide a means of determining the listening address" do
         @server.address.should == "127.0.0.1"
     end
-    
+
     it "should provide a means of determining the listening port" do
         @server.port.should == 31337
     end
+
+    it "should allow for multiple configurations, each handling different indirections" do
+        Puppet::Indirector::Indirection.stubs(:model).returns mock('indirection')
+
+        @server2 = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337)
+        @server.register(:foo, :bar)
+        @server2.register(:foo, :xyzzy)
+        @server.unregister(:foo, :bar)
+        @server2.unregister(:foo, :xyzzy)
+        lambda { @server.unregister(:xyzzy) }.should raise_error(ArgumentError)
+        lambda { @server2.unregister(:bar) }.should raise_error(ArgumentError)
+    end  
+
+    describe "when managing xmlrpc registrations" do
+        before do
+            Puppet::Network::Handler.stubs(:handler).returns mock('xmlrpc_handler')
+        end
+
+        it "should allow registering an xmlrpc handler by specifying its namespace" do
+            lambda { @server.register_xmlrpc(:foo) }.should_not raise_error
+        end
+
+        it "should require that the xmlrpc namespace be valid" do
+            Puppet::Network::Handler.stubs(:handler).returns nil
+
+            lambda { @server.register_xmlrpc(:foo) }.should raise_error(ArgumentError)
+        end
+
+        it "should require at least one namespace" do
+            lambda { @server.register_xmlrpc() }.should raise_error(ArgumentError)
+        end
+
+        it "should allow multiple namespaces to be registered at once" do
+            lambda { @server.register_xmlrpc(:foo, :bar) }.should_not raise_error
+        end
+
+        it "should allow the use of namespaces to specify which are no longer accessible to clients" do
+            @server.register_xmlrpc(:foo, :bar)
+        end
+
+        it "should leave other namespaces accessible to clients when turning off xmlrpc namespaces" do
+            @server.register_xmlrpc(:foo, :bar)
+            @server.unregister_xmlrpc(:foo)
+            lambda { @server.unregister_xmlrpc(:bar)}.should_not raise_error
+        end
+
+        it "should allow specifying numerous namespaces which are to be no longer accessible to clients" do
+            @server.register_xmlrpc(:foo, :bar)
+            lambda { @server.unregister_xmlrpc(:foo, :bar) }.should_not raise_error
+        end
+
+        it "should not turn off any indirections if given unknown namespaces to turn off" do
+            @server.register_xmlrpc(:foo, :bar)
+            lambda { @server.unregister_xmlrpc(:foo, :bar, :baz) }.should raise_error(ArgumentError)
+            lambda { @server.unregister_xmlrpc(:foo, :bar) }.should_not raise_error
+        end
+
+        it "should not allow turning off unknown namespaces" do
+            @server.register_xmlrpc(:foo, :bar)
+            lambda { @server.unregister_xmlrpc(:baz) }.should raise_error(ArgumentError)
+        end
+
+        it "should disable client access immediately when turning off namespaces" do
+            @server.register_xmlrpc(:foo, :bar)
+            @server.unregister_xmlrpc(:foo)    
+            lambda { @server.unregister_xmlrpc(:foo) }.should raise_error(ArgumentError)
+        end
+
+        it "should allow turning off all namespaces at once" do
+            @server.register_xmlrpc(:foo, :bar)
+            @server.unregister_xmlrpc
+            [ :foo, :bar, :baz].each do |indirection|
+                lambda { @server.unregister_xmlrpc(indirection) }.should raise_error(ArgumentError)
+            end
+        end
+    end
 end
 
 describe Puppet::Network::Server, "when listening is off" do
@@ -192,15 +277,15 @@ describe Puppet::Network::Server, "when listening is off" do
     it "should indicate that it is not listening" do
         @server.should_not be_listening
     end  
-  
+
     it "should not allow listening to be turned off" do
-        Proc.new { @server.unlisten }.should raise_error(RuntimeError)
+        lambda { @server.unlisten }.should raise_error(RuntimeError)
     end
-  
+
     it "should allow listening to be turned on" do
-        Proc.new { @server.listen }.should_not raise_error
+        lambda { @server.listen }.should_not raise_error
     end
-    
+
 end
 
 describe Puppet::Network::Server, "when listening is on" do
@@ -215,26 +300,30 @@ describe Puppet::Network::Server, "when listening is on" do
         @server.stubs(:http_server).returns(@mock_http_server)
         @server.listen
     end
-  
+
     it "should indicate that it is listening" do
         @server.should be_listening
     end
-    
+
     it "should not allow listening to be turned on" do
-        Proc.new { @server.listen }.should raise_error(RuntimeError)
+        lambda { @server.listen }.should raise_error(RuntimeError)
     end
-  
+
     it "should allow listening to be turned off" do
-        Proc.new { @server.unlisten }.should_not raise_error
+        lambda { @server.unlisten }.should_not raise_error
     end
 end
- 
+
 describe Puppet::Network::Server, "when listening is being turned on" do
     before do
         @mock_http_server_class = mock('http server class')
         Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class)
         Puppet.stubs(:[]).with(:servertype).returns(:suparserver)
-        @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337, :handlers => [:node])
+
+        Puppet::Indirector::Indirection.stubs(:model).returns mock('indirection')
+        Puppet::Network::Handler.stubs(:handler).returns mock('xmlrpc_handler')
+
+        @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337, :handlers => [:node], :xmlrpc_handlers => [:master])
         @mock_http_server = mock('http server')
         @mock_http_server.stubs(:listen)
     end
@@ -250,37 +339,46 @@ describe Puppet::Network::Server, "when listening is being turned on" do
         @mock_http_server.expects(:listen)
         @server.listen
     end
-    
+
     it "should pass the listening address to the HTTP server" do
        @server.stubs(:http_server).returns(@mock_http_server)
        @mock_http_server.expects(:listen).with do |args|
-            args[:address] == '127.0.0.1'
+           args[:address] == '127.0.0.1'
        end
        @server.listen
     end
-    
+
     it "should pass the listening port to the HTTP server" do
-      @server.stubs(:http_server).returns(@mock_http_server)
-      @mock_http_server.expects(:listen).with do |args|
-           args[:port] == 31337
-      end
-      @server.listen
-    end
-    
-    it "should pass a list of handlers to the HTTP server" do
-      @server.stubs(:http_server).returns(@mock_http_server)
-      @mock_http_server.expects(:listen).with do |args|
-           args[:handlers] == [ :node ]
-      end
-      @server.listen
-    end
-    
+        @server.stubs(:http_server).returns(@mock_http_server)
+        @mock_http_server.expects(:listen).with do |args|
+            args[:port] == 31337
+        end
+        @server.listen
+    end
+
+    it "should pass a list of REST handlers to the HTTP server" do
+        @server.stubs(:http_server).returns(@mock_http_server)
+        @mock_http_server.expects(:listen).with do |args|
+            args[:handlers] == [ :node ]
+        end
+        @server.listen
+    end
+
+    it "should pass a list of XMLRPC handlers to the HTTP server" do
+        @server.stubs(:http_server).returns(@mock_http_server)
+        @mock_http_server.expects(:listen).with do |args|
+            p args
+            args[:xmlrpc_handlers] == [ :master ]
+        end
+        @server.listen
+    end
+
     it "should pass a list of protocols to the HTTP server" do
-      @server.stubs(:http_server).returns(@mock_http_server)
-      @mock_http_server.expects(:listen).with do |args|
-           args[:protocols] == [ :rest ]
-      end
-      @server.listen      
+        @server.stubs(:http_server).returns(@mock_http_server)
+        @mock_http_server.expects(:listen).with do |args|
+           args[:protocols] == [ :rest, :xmlrpc ]
+        end
+        @server.listen      
     end
 end
 
@@ -302,12 +400,14 @@ describe Puppet::Network::Server, "when listening is being turned off" do
     end
 
     it "should not allow for indirections to be turned off" do
+        Puppet::Indirector::Indirection.stubs(:model).returns mock('indirection')
+
         @server.register(:foo)
-        Proc.new { @server.unregister(:foo) }.should raise_error(RuntimeError) 
+        lambda { @server.unregister(:foo) }.should raise_error(RuntimeError) 
     end
 end
 
-                
+
 describe Class.new, "put these somewhere" do
     it "should have the ability to use a class-level from_ hook (from_yaml, from_text, etc.) that can be called, based on content-type header, to allow for different deserializations of an object" 
     it "should allow from_* on the inbound :data packet (look at its content_type) when doing a PUT/.new.save"