The REST terminus now uses the content-type and http result codes.

Things are currently broken -- this is a checkpoint commit
so that it's safe to make mistakes and will probably be
removed.

Signed-off-by: Luke Kanies <luke@madstop.com>

The REST terminus now completely does error handling and serialization.

The Integration tests are still completely broken.

Signed-off-by: Luke Kanies <luke@madstop.com>

The Mongrel and Webrick rest handlers no longer yaml-encode exceptions.

They just store the exceptions in plain text in the message body.

They also set the status to 400, rather than 404.

Signed-off-by: Luke Kanies <luke@madstop.com>

8 files changed, 232 insertions, 300 deletions

diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index 6dbdada4d..8889dbc3e 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -3,58 +3,47 @@ require 'uri'
 
 # Access objects via REST
 class Puppet::Indirector::REST < Puppet::Indirector::Terminus
+    # Figure out the content type, turn that into a format, and use the format
+    # to extract the body of the response.
+    def deserialize(response)
+        # Raise the http error if we didn't get a 'success' of some kind.
+        response.error! unless response.code =~ /^2/
+
+        # Convert the response to a deserialized object.
+        model.convert_from(response['content-type'], response.body)
+    end
+
     # Provide appropriate headers.
     def headers
         {"Accept" => model.supported_formats.join(", ")}
     end
+  
+    def network
+        Puppet::Network::HttpPool.http_instance(Puppet[:server], Puppet[:masterport].to_i)
+    end
 
     def rest_connection_details
         { :host => Puppet[:server], :port => Puppet[:masterport].to_i }
     end
-
-    def network_fetch(path)
-        network.get("/#{path}", headers).body
-    end
-    
-    def network_delete(path)
-        network.delete("/#{path}", headers).body
-    end
-    
-    def network_put(path, data)
-        network.put("/#{path}", data, headers).body
-    end
     
     def find(request)
-        network_result = network_fetch("#{indirection.name}/#{request.key}")
-        raise YAML.load(network_result) if exception?(network_result)
-        indirection.model.from_yaml(network_result)
+        deserialize network.get("/#{indirection.name}/#{request.key}", headers)
     end
     
     def search(request)
-        network_results = network_fetch("#{indirection.name}s/#{request.key}")
-        raise YAML.load(network_results) if exception?(network_results)
-        YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) }
+        if request.key
+            path = "/#{indirection.name}/#{request.key}"
+        else
+            path = "/#{indirection.name}"
+        end
+        deserialize network.get(path, headers)
     end
     
     def destroy(request)
-        network_result = network_delete("#{indirection.name}/#{request.key}")
-        raise YAML.load(network_result) if exception?(network_result)
-        YAML.load(network_result.to_s)      
+        deserialize network.delete("/#{indirection.name}/#{request.key}", headers)
     end
     
     def save(request)
-        network_result = network_put("#{indirection.name}/", request.instance.to_yaml)
-        raise YAML.load(network_result) if exception?(network_result)
-        indirection.model.from_yaml(network_result)
-    end
-    
-  private
-  
-    def network
-        Puppet::Network::HttpPool.http_instance(rest_connection_details[:host], rest_connection_details[:port])
-    end
-  
-    def exception?(yaml_string)
-        yaml_string =~ %r{--- !ruby/exception}
+        deserialize network.put("/#{indirection.name}/", request.instance.render, headers)
     end
 end
diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb
index ab828ad12..b569bd696 100644
--- a/lib/puppet/network/format_handler.rb
+++ b/lib/puppet/network/format_handler.rb
@@ -15,6 +15,10 @@ module Puppet::Network::FormatHandler
             send("from_%s" % format, data)
         end
 
+        def default_format
+            supported_formats[0]
+        end
+
         def support_format?(name)
             respond_to?("from_%s" % name) and instance_methods.include?("to_%s" % name)
         end
@@ -29,8 +33,13 @@ module Puppet::Network::FormatHandler
     end
 
     module InstanceMethods
-        def render_to(format)
-            raise ArgumentError, "Format %s not supported" % format unless support_format?(format)
+        def render(format = nil)
+            if format
+                raise ArgumentError, "Format %s not supported" % format unless support_format?(format)
+            else
+                format = self.class.default_format
+            end
+
             send("to_%s" % format)
         end
 
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index 3c14c8a40..a862bb85e 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -58,8 +58,8 @@ module Puppet::Network::HTTP::Handler
         object.save(args)
     end
 
-    def do_exception(request, response, exception, status=404)
-        encode_result(request, response, exception.to_yaml, status)
+    def do_exception(request, response, exception, status=400)
+        encode_result(request, response, exception.to_s, status)
     end
 
     def find_model_for_handler(handler)
diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb
index 9f531352a..63291dc5c 100755
--- a/spec/integration/indirector/rest.rb
+++ b/spec/integration/indirector/rest.rb
@@ -144,7 +144,7 @@ describe Puppet::Indirector::REST do
                     Puppet::TestIndirectedFoo.search('bar').collect { |i| i.value }.should == @model_instances.collect { |i| i.value }
                 end
             end
-        
+
             describe "when no matching model instance can be found" do
                 before :each do
                     @mock_model = stub('faked model', :find => nil)
@@ -155,7 +155,7 @@ describe Puppet::Indirector::REST do
                     Puppet::TestIndirectedFoo.find('bar').should be_nil
                 end
             end
-        
+
             describe "when an exception is encountered in looking up a model instance" do
                 before :each do
                     @mock_model = stub('faked model')
diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb
index 4a3b9d2d0..350b8a1d0 100755
--- a/spec/unit/indirector/rest.rb
+++ b/spec/unit/indirector/rest.rb
@@ -12,34 +12,23 @@ describe "a REST http call", :shared => true do
         lambda { @searcher.send(@method) }.should raise_error(ArgumentError)
     end
 
-    it "should use the Http Pool with the remote server and port looked up from the REST terminus" do
-        @searcher.expects(:rest_connection_details).returns(@details)
-
-        conn = mock 'connection'
-        result = stub 'result', :body => "body"
-        conn.stubs(:put).returns result
-        conn.stubs(:delete).returns result
-        conn.stubs(:get).returns result
-        Puppet::Network::HttpPool.expects(:http_instance).with(@details[:host], @details[:port]).returns conn
-        @searcher.send(@method, *@arguments)
-    end
-
-    it "should return the results of the request" do
+    it "should return the results of deserializing the response to the request" do
         conn = mock 'connection'
-        result = stub 'result', :body => "result"
-        conn.stubs(:put).returns result
-        conn.stubs(:delete).returns result
-        conn.stubs(:get).returns result
+        conn.stubs(:put).returns @response
+        conn.stubs(:delete).returns @response
+        conn.stubs(:get).returns @response
         Puppet::Network::HttpPool.stubs(:http_instance).returns conn
 
-        @searcher.send(@method, *@arguments).should == 'result'                
+        @searcher.expects(:deserialize).with(@response).returns "myobject"
+
+        @searcher.send(@method, *@arguments).should == 'myobject'
     end        
 end
 
 describe Puppet::Indirector::REST do
     before do
         Puppet::Indirector::Terminus.stubs(:register_terminus_class)
-        @model = stub('model', :supported_formats => %w{})
+        @model = stub('model', :supported_formats => %w{}, :convert_from => nil)
         @instance = stub('model instance')
         @indirection = stub('indirection', :name => :mystuff, :register_terminus_type => nil, :model => @model)
         Puppet::Indirector::Indirection.stubs(:instance).returns(@indirection)
@@ -50,311 +39,240 @@ describe Puppet::Indirector::REST do
             end
         end
 
+        @response = stub('mock response', :body => 'result', :code => "200")
+        @response.stubs(:[]).with('content-type').returns "text/plain"
+
         @searcher = @rest_class.new
     end
 
-    describe "when configuring the REST http call" do
-        before do
-            Puppet.settings.stubs(:value).returns("rest_testing")
-        end
+    describe "when deserializing responses" do
+        it "should return the results of converting from the format specified by the content-type header" do
+            @model.expects(:convert_from).with("myformat", "mydata").returns "myobject"
 
-        it "should return the :server setting as the host" do
-            Puppet.settings.expects(:value).with(:server).returns "myserver"
-            @searcher.rest_connection_details[:host].should == "myserver"
-        end
+            response = mock 'response'
+            response.expects(:[]).with("content-type").returns "myformat"
+            response.expects(:body).returns "mydata"
+            response.stubs(:code).returns "200"
 
-        it "should return the :masterport (as an Integer) as the port" do
-            Puppet.settings.expects(:value).with(:masterport).returns "1234"
-            @searcher.rest_connection_details[:port].should == 1234
-        end
-    end
-
-    describe "when doing a network fetch" do
-        before :each do
-            Net::HTTP.stubs(:start).returns('result')
-            @details = { :host => '127.0.0.1', :port => 34343 }
-            @searcher.stubs(:rest_connection_details).returns(@details)
-
-            @method = :network_fetch
-            @arguments = "foo"
+            @searcher.deserialize(response)
         end
 
-        it_should_behave_like "a REST http call"
+        it "should fail if the response is not a success" do
+            @model.expects(:convert_from).never
 
-        it "should use the GET http method" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = mock('mock http connection', :get => @mock_result)
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_fetch('foo')
-        end
+            response = mock 'response'
+            response.expects(:code).returns "300"
+            response.expects(:error!).raises ArgumentError
 
-        it "should use the provided path" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = stub('mock http connection')
-            @mock_connection.expects(:get).with { |path, args| path == '/foo' }.returns(@mock_result)
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_fetch('foo')
-        end
-
-        it "should provide an Accept header containing the list of supported formats joined with commas" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = stub('mock http connection')
-            @mock_connection.expects(:get).with { |path, args| args["Accept"] == "supported, formats" }.returns(@mock_result)
-
-            @searcher.model.expects(:supported_formats).returns %w{supported formats}
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_fetch('foo')
-        end
-    end
-
-    describe "when doing a network delete" do
-        before :each do
-            Net::HTTP.stubs(:start).returns('result')
-            @details = { :host => '127.0.0.1', :port => 34343 }
-            @searcher.stubs(:rest_connection_details).returns(@details)
-
-            @method = :network_delete
-            @arguments = "foo"
-        end
-
-        it_should_behave_like "a REST http call"
-
-        it "should use the DELETE http method" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = mock('mock http connection')
-            @mock_connection.expects(:delete).with { |path, args| path == '/foo' }.returns @mock_result
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_delete('foo')
-        end
-
-        it "should provide an Accept header containing the list of supported formats joined with commas" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = mock('mock http connection')
-            @mock_connection.expects(:delete).with { |path, args| args['Accept'] == "supported, formats" }.returns @mock_result
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.model.expects(:supported_formats).returns %w{supported formats}
-            @searcher.network_delete('foo')
+            lambda { @searcher.deserialize(response) }.should raise_error(ArgumentError)
         end
     end
 
-    describe "when doing a network put" do
-        before :each do
-            Net::HTTP.stubs(:start).returns('result')
-            @details = { :host => '127.0.0.1', :port => 34343 }
-            @data = { :foo => 'bar' }
-            @searcher.stubs(:rest_connection_details).returns(@details)
-
-            @method = :network_put
-            @arguments = ["foo", @data]
-        end
-
-        it_should_behave_like "a REST http call"
-
-        it "should use the PUT http method" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = mock('mock http connection', :put => @mock_result)
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_put('foo', @data)
-        end
-
-        it "should use the provided path" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = stub('mock http connection')
-            @mock_connection.expects(:put).with {|path, data, args| path == '/foo' }.returns(@mock_result)
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_put('foo', @data)                
-        end
-
-        it "should use the provided data" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = stub('mock http connection')
-            @mock_connection.expects(:put).with {|path, data, args| data == @data }.returns(@mock_result)
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.network_put('foo', @data)                                
+    describe "when creating an HTTP client" do
+        before do
+            Puppet.settings.stubs(:value).returns("rest_testing")
         end
 
-        it "should provide an Accept header containing the list of supported formats joined with commas" do
-            @mock_result = stub('mock result', :body => 'result')
-            @mock_connection = mock('mock http connection')
-            @mock_connection.expects(:put).with { |path, data, args| args['Accept'] == "supported, formats" }.returns @mock_result
-            @searcher.stubs(:network).returns(@mock_connection)
-            @searcher.model.expects(:supported_formats).returns %w{supported formats}
-            @searcher.network_put('foo', @data)
+        it "should use the :server setting as the host and the :masterport setting (as an Integer) as the port" do
+            Puppet.settings.expects(:value).with(:server).returns "myserver"
+            Puppet.settings.expects(:value).with(:masterport).returns "1234"
+            Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 1234).returns "myconn"
+            @searcher.network.should == "myconn"
         end
     end
 
     describe "when doing a find" do
         before :each do
-            @result = { :foo => 'bar'}.to_yaml
-            @searcher.stubs(:network_fetch).returns(@result)    # neuter the network connection
-            @model.stubs(:from_yaml).returns(@instance)
+            @connection = stub('mock http connection', :get => @response)
+            @searcher.stubs(:network).returns(@connection)    # neuter the network connection
 
             @request = stub 'request', :key => 'foo'
         end
 
-        it "should look up the model instance over the network" do
-            @searcher.expects(:network_fetch).returns(@result)
+        it "should call the GET http method on a network connection" do
+            @searcher.expects(:network).returns @connection
+            @connection.expects(:get).returns @response
             @searcher.find(@request)
         end
 
-        it "should look up the model instance using the named indirection" do
-            @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result)
-            @searcher.find(@request)
-        end
+        it "should deserialize and return the http response" do
+            @connection.expects(:get).returns @response
+            @searcher.expects(:deserialize).with(@response).returns "myobject"
 
-        it "should look up the model instance using the provided key" do
-            @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result)
-            @searcher.find(@request)
-        end
+            @searcher.find(@request).should == 'myobject'
+        end        
 
-        it "should deserialize result data to a Model instance" do
-            @model.expects(:from_yaml)
+        it "should use the indirection name and request key to create the path" do
+            should_path = "/%s/%s" % [@indirection.name.to_s, "foo"]
+            @connection.expects(:get).with { |path, args| path == should_path }.returns(@response)
             @searcher.find(@request)
         end
 
-        it "should return the deserialized Model instance" do
-            @searcher.find(@request).should == @instance         
+        it "should provide an Accept header containing the list of supported formats joined with commas" do
+            @connection.expects(:get).with { |path, args| args["Accept"] == "supported, formats" }.returns(@response)
+
+            @searcher.model.expects(:supported_formats).returns %w{supported formats}
+            @searcher.find(@request)
         end
 
-        it "should return nil when deserialized model instance is nil" do
-            @model.stubs(:from_yaml).returns(nil)
-            @searcher.find(@request).should be_nil
+        it "should deserialize and return the network response" do
+            @searcher.expects(:deserialize).with(@response).returns @instance
+            @searcher.find(@request).should equal(@instance)
         end
 
-        it "should generate an error when result data deserializes improperly" do
-            @model.stubs(:from_yaml).raises(ArgumentError)
+        it "should generate an error when result data deserializes fails" do
+            @searcher.expects(:deserialize).raises(ArgumentError)
             lambda { @searcher.find(@request) }.should raise_error(ArgumentError)
         end
-
-        it "should generate an error when result data specifies an error" do
-            @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml)
-            lambda { @searcher.find(@request) }.should raise_error(RuntimeError)                
-        end            
     end
 
     describe "when doing a search" do
         before :each do
-            @result = [1, 2].to_yaml
-            @searcher.stubs(:network_fetch).returns(@result)
-            @model.stubs(:from_yaml).returns(@instance)
+            @connection = stub('mock http connection', :get => @response)
+            @searcher.stubs(:network).returns(@connection)    # neuter the network connection
 
             @request = stub 'request', :key => 'foo'
         end
 
-        it "should look up the model data over the network" do
-            @searcher.expects(:network_fetch).returns(@result)
+        it "should call the GET http method on a network connection" do
+            @searcher.expects(:network).returns @connection
+            @connection.expects(:get).returns @response
             @searcher.search(@request)
         end
 
-        it "should look up the model instance using the plural of the named indirection" do
-            @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}s/} }.returns(@result)
+        it "should deserialize and return the http response" do
+            @connection.expects(:get).returns @response
+            @searcher.expects(:deserialize).with(@response).returns "myobject"
+
+            @searcher.search(@request).should == 'myobject'
+        end        
+
+        it "should use the indirection name as the path if there is no request key" do
+            should_path = "/%s" % [@indirection.name.to_s]
+            @request.stubs(:key).returns nil
+            @connection.expects(:get).with { |path, args| path == should_path }.returns(@response)
             @searcher.search(@request)
         end
 
-        it "should look up the model instance using the provided key" do
-            @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result)
+        it "should use the indirection name and request key to create the path if the request key is set" do
+            should_path = "/%s/%s" % [@indirection.name.to_s, "foo"]
+            @connection.expects(:get).with { |path, args| path == should_path }.returns(@response)
             @searcher.search(@request)
         end
 
-        it "should deserialize result data into a list of Model instances" do
-            @model.expects(:from_yaml).at_least(2)
+        it "should provide an Accept header containing the list of supported formats joined with commas" do
+            @connection.expects(:get).with { |path, args| args["Accept"] == "supported, formats" }.returns(@response)
+
+            @searcher.model.expects(:supported_formats).returns %w{supported formats}
             @searcher.search(@request)
         end
 
-        it "should generate an error when result data deserializes improperly" do
-            @model.stubs(:from_yaml).raises(ArgumentError)
-            lambda { @searcher.search(@request) }.should raise_error(ArgumentError)                
+        it "should deserialize and return the network response" do
+            @searcher.expects(:deserialize).with(@response).returns @instance
+            @searcher.search(@request).should equal(@instance)
         end
 
-        it "should generate an error when result data specifies an error" do
-            @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml)
-            lambda { @searcher.search(@request) }.should raise_error(RuntimeError)                
-        end         
+        it "should generate an error when result data deserializes fails" do
+            @searcher.expects(:deserialize).raises(ArgumentError)
+            lambda { @searcher.search(@request) }.should raise_error(ArgumentError)
+        end
     end        
 
     describe "when doing a destroy" do
         before :each do
-            @result = true.to_yaml
-            @searcher.stubs(:network_delete).returns(@result)    # neuter the network connection
-            @model.stubs(:from_yaml).returns(@instance)
+            @connection = stub('mock http connection', :delete => @response)
+            @searcher.stubs(:network).returns(@connection)    # neuter the network connection
 
             @request = stub 'request', :key => 'foo'
         end
 
-        it "should look up the model instance over the network" do
-            @searcher.expects(:network_delete).returns(@result)
+        it "should call the DELETE http method on a network connection" do
+            @searcher.expects(:network).returns @connection
+            @connection.expects(:delete).returns @response
             @searcher.destroy(@request)
         end
 
-        it "should look up the model instance using the named indirection" do
-            @searcher.expects(:network_delete).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result)
-            @searcher.destroy(@request)
-        end
+        it "should deserialize and return the http response" do
+            @connection.expects(:delete).returns @response
+            @searcher.expects(:deserialize).with(@response).returns "myobject"
+
+            @searcher.destroy(@request).should == 'myobject'
+        end        
 
-        it "should look up the model instance using the provided key" do
-            @searcher.expects(:network_delete).with {|path| path =~ %r{/foo$} }.returns(@result)
+        it "should use the indirection name and request key to create the path" do
+            should_path = "/%s/%s" % [@indirection.name.to_s, "foo"]
+            @connection.expects(:delete).with { |path, args| path == should_path }.returns(@response)
             @searcher.destroy(@request)
         end
 
-        it "should deserialize result data" do
-            YAML.expects(:load).with(@result)
+        it "should provide an Accept header containing the list of supported formats joined with commas" do
+            @connection.expects(:delete).with { |path, args| args["Accept"] == "supported, formats" }.returns(@response)
+
+            @searcher.model.expects(:supported_formats).returns %w{supported formats}
             @searcher.destroy(@request)
         end
 
-        it "should return deserialized result data" do
-            @searcher.destroy(@request).should == true
+        it "should deserialize and return the network response" do
+            @searcher.expects(:deserialize).with(@response).returns @instance
+            @searcher.destroy(@request).should equal(@instance)
         end
 
-        it "should generate an error when result data specifies an error" do
-            @searcher.stubs(:network_delete).returns(RuntimeError.new("bogus").to_yaml)
-            lambda { @searcher.destroy(@request) }.should raise_error(RuntimeError)                
-        end            
+        it "should generate an error when result data deserializes fails" do
+            @searcher.expects(:deserialize).raises(ArgumentError)
+            lambda { @searcher.destroy(@request) }.should raise_error(ArgumentError)
+        end
     end
 
     describe "when doing a save" do
         before :each do
-            @result = { :foo => 'bar'}.to_yaml
-            @searcher.stubs(:network_put).returns(@result)    # neuter the network connection
-            @model.stubs(:from_yaml).returns(@instance)
+            @connection = stub('mock http connection', :put => @response)
+            @searcher.stubs(:network).returns(@connection)    # neuter the network connection
 
+            @instance = stub 'instance', :render => "mydata"
             @request = stub 'request', :instance => @instance
         end
 
-        it "should save the model instance over the network" do
-            @searcher.expects(:network_put).returns(@result)
+        it "should call the PUT http method on a network connection" do
+            @searcher.expects(:network).returns @connection
+            @connection.expects(:put).returns @response
             @searcher.save(@request)
         end
 
-        it "should save the model instance using the named indirection" do
-            @searcher.expects(:network_put).with do |path, data| 
-                path =~ %r{^#{@indirection.name.to_s}/} and 
-                data == @instance.to_yaml 
-            end.returns(@result)
+        it "should use the indirection name as the path for the request" do
+            @connection.expects(:put).with { |path, data, args| path == "/#{@indirection.name.to_s}/" }.returns @response
+
             @searcher.save(@request)
         end
 
-        it "should deserialize result data to a Model instance" do
-            @model.expects(:from_yaml)
+        it "should serialize the instance using the default format and pass the result as the body of the request" do
+            @instance.expects(:render).returns "serial_instance"
+            @connection.expects(:put).with { |path, data, args| data == "serial_instance" }.returns @response
+
             @searcher.save(@request)
         end
 
-        it "should return the resulting deserialized Model instance" do
-            @searcher.save(@request).should == @instance         
+        it "should deserialize and return the http response" do
+            @connection.expects(:put).returns @response
+            @searcher.expects(:deserialize).with(@response).returns "myobject"
+
+            @searcher.save(@request).should == 'myobject'
+        end        
+
+        it "should provide an Accept header containing the list of supported formats joined with commas" do
+            @connection.expects(:put).with { |path, data, args| args["Accept"] == "supported, formats" }.returns(@response)
+
+            @searcher.model.expects(:supported_formats).returns %w{supported formats}
+            @searcher.save(@request)
         end
 
-        it "should return nil when deserialized model instance is nil" do
-            @model.stubs(:from_yaml).returns(nil)
-            @searcher.save(@request).should be_nil
+        it "should deserialize and return the network response" do
+            @searcher.expects(:deserialize).with(@response).returns @instance
+            @searcher.save(@request).should equal(@instance)
         end
 
-        it "should generate an error when result data deserializes improperly" do
-            @model.stubs(:from_yaml).raises(ArgumentError)
+        it "should generate an error when result data deserializes fails" do
+            @searcher.expects(:deserialize).raises(ArgumentError)
             lambda { @searcher.save(@request) }.should raise_error(ArgumentError)
         end
-
-        it "should generate an error when result data specifies an error" do
-            @searcher.stubs(:network_put).returns(RuntimeError.new("bogus").to_yaml)
-            lambda { @searcher.save(@request) }.should raise_error(RuntimeError)                
-        end            
     end
 end
diff --git a/spec/unit/network/format_handler.rb b/spec/unit/network/format_handler.rb
index a63c26b08..3e0571eb6 100755
--- a/spec/unit/network/format_handler.rb
+++ b/spec/unit/network/format_handler.rb
@@ -61,6 +61,11 @@ describe Puppet::Network::FormatHandler do
         FormatTester.supported_formats.should == %w{good}
     end
 
+    it "should return the first format as the default format" do
+        FormatTester.expects(:supported_formats).returns %w{one two}
+        FormatTester.default_format.should == "one"
+    end
+
     describe "when an instance" do
         it "should be able to test whether a format is supported" do
             FormatTester.new.should respond_to(:support_format?)
@@ -75,19 +80,26 @@ describe Puppet::Network::FormatHandler do
         end
 
         it "should be able to convert to a given format" do
-            FormatTester.new.should respond_to(:render_to)
+            FormatTester.new.should respond_to(:render)
         end
 
         it "should fail if asked to convert to an unsupported format" do
             tester = FormatTester.new
             tester.expects(:support_format?).with(:nope).returns false
-            lambda { tester.render_to(:nope) }.should raise_error(ArgumentError)
+            lambda { tester.render(:nope) }.should raise_error(ArgumentError)
         end
 
         it "should call the format-specific converter when asked to convert to a given format" do
             tester = FormatTester.new
             tester.expects(:to_good)
-            tester.render_to(:good)
+            tester.render(:good)
+        end
+
+        it "should render to the default format if no format is provided when rendering" do
+            FormatTester.expects(:default_format).returns "foo"
+            tester = FormatTester.new
+            tester.expects(:to_foo)
+            tester.render
         end
     end
 end
diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb
index de5254a7a..9f35c199c 100755
--- a/spec/unit/network/http/mongrel/rest.rb
+++ b/spec/unit/network/http/mongrel/rest.rb
@@ -110,17 +110,17 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
 
     it "should fail if the HTTP method isn't supported" do
         @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'POST', Mongrel::Const::REQUEST_PATH => '/foo'})
-        @mock_response.expects(:start).with(404)
+        @mock_response.expects(:start).with(400)
         @handler.process(@mock_request, @mock_response)
     end
 
     it "should fail if the request's pluralization is wrong" do
         @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'DELETE', Mongrel::Const::REQUEST_PATH => '/foos/key'})
-        @mock_response.expects(:start).with(404)
+        @mock_response.expects(:start).with(400)
         @handler.process(@mock_request, @mock_response)
 
         @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'PUT', Mongrel::Const::REQUEST_PATH => '/foos/key'})
-        @mock_response.expects(:start).with(404)
+        @mock_response.expects(:start).with(400)
         @handler.process(@mock_request, @mock_response)
     end
 
@@ -128,10 +128,18 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         @mock_request.stubs(:params).returns({  Mongrel::Const::REQUEST_METHOD => 'GET', 
                                                 Mongrel::Const::REQUEST_PATH => '/bar/key',
                                                 'QUERY_STRING' => '' })
-        @mock_response.expects(:start).with(404)
+        @mock_response.expects(:start).with(400)
         @handler.process(@mock_request, @mock_response)
     end
 
+    it "should serialize a controller exception if the request fails" do
+        @mock_request.stubs(:params).raises(ArgumentError, "This is a failure")
+        body = mock 'body'
+        @mock_response.expects(:start).with(400).yields("head", body)
+        body.expects(:write).with("This is a failure")
+        @handler.process(@mock_request, @mock_response)        
+    end
+
     describe "and determining the request parameters", :shared => true do
         confine "Mongrel is not available" => Puppet.features.mongrel?
 
@@ -206,7 +214,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
 
         it "should fail to find model if key is not specified" do
             @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'GET', Mongrel::Const::REQUEST_PATH => '/foo'})
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)
         end
 
@@ -236,7 +244,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by find" do
            setup_find_request
            @mock_model_class.expects(:find).raises(ArgumentError) 
-           @mock_response.expects(:start).with(404)
+           @mock_response.expects(:start).with(400)
            @handler.process(@mock_request, @mock_response)        
         end
     end
@@ -246,7 +254,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
 
         it "should fail to destroy model if key is not specified" do
             @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'DELETE', Mongrel::Const::REQUEST_PATH => '/foo'})
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)
         end
 
@@ -283,7 +291,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by destroy" do
             setup_destroy_request
             @mock_model_class.expects(:destroy).raises(ArgumentError) 
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)                 
         end
     end
@@ -294,7 +302,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         it "should fail to save model if data is not specified" do
             @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'PUT', Mongrel::Const::REQUEST_PATH => '/foo'})
             @mock_request.stubs(:body).returns('')
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)
         end
 
@@ -323,7 +331,7 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by save" do
             setup_save_request
             @mock_model_instance.expects(:save).raises(ArgumentError) 
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)                         
         end
     end
@@ -364,14 +372,8 @@ describe "Puppet::Network::HTTP::MongrelREST", "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by search" do
             setup_search_request
             @mock_model_class.expects(:search).raises(ArgumentError) 
-            @mock_response.expects(:start).with(404)
+            @mock_response.expects(:start).with(400)
             @handler.process(@mock_request, @mock_response)                
         end
     end    
-
-    it "should serialize a controller exception if the request fails" do
-        setup_bad_request     
-        @mock_response.expects(:start).with(404)
-        @handler.process(@mock_request, @mock_response)        
-    end
 end
diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb
index 17d47e54c..782f99045 100755
--- a/spec/unit/network/http/webrick/rest.rb
+++ b/spec/unit/network/http/webrick/rest.rb
@@ -107,31 +107,39 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
     it "should fail if the HTTP method isn't supported" do
         @mock_request.stubs(:request_method).returns('POST')
         @mock_request.stubs(:path).returns('/foo')
-        @mock_response.expects(:status=).with(404)
+        @mock_response.expects(:status=).with(400)
         @handler.process(@mock_request, @mock_response)            
     end
 
     it "should fail if delete request's pluralization is wrong" do
         @mock_request.stubs(:request_method).returns('DELETE')
         @mock_request.stubs(:path).returns('/foos/key')
-        @mock_response.expects(:status=).with(404)
+        @mock_response.expects(:status=).with(400)
         @handler.process(@mock_request, @mock_response)            
     end
 
     it "should fail if put request's pluralization is wrong" do 
         @mock_request.stubs(:request_method).returns('PUT')
         @mock_request.stubs(:path).returns('/foos/key')
-        @mock_response.expects(:status=).with(404)
+        @mock_response.expects(:status=).with(400)
         @handler.process(@mock_request, @mock_response)            
     end
 
     it "should fail if the request is for an unknown path" do
         @mock_request.stubs(:request_method).returns('GET')
         @mock_request.stubs(:path).returns('/bar/key')
-        @mock_response.expects(:status=).with(404)
+        @mock_response.expects(:status=).with(400)
         @handler.process(@mock_request, @mock_response)            
     end
 
+    it "should set the response status to 400 and the body to the exception message if the request fails" do
+        @mock_request.stubs(:request_method).raises(ArgumentError, "This is a failure")
+        @mock_request.stubs(:path).returns('/foos')
+        @mock_response.expects(:status=).with(400)
+        @mock_response.expects(:body=).with("This is a failure")
+        @handler.process(@mock_request, @mock_response)        
+    end
+
     describe "and determining the request parameters" do
         it "should include the HTTP request parameters" do
             @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy)
@@ -172,7 +180,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should fail to find model if key is not specified" do
             @mock_request.stubs(:request_method).returns('GET')
             @mock_request.stubs(:path).returns('/foo')
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)            
         end
 
@@ -202,7 +210,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by find" do
            setup_find_request
            @mock_model_class.expects(:find).raises(ArgumentError) 
-           @mock_response.expects(:status=).with(404)
+           @mock_response.expects(:status=).with(400)
            @handler.process(@mock_request, @mock_response)        
         end
     end
@@ -211,7 +219,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should fail to destroy model if key is not specified" do
             @mock_request.stubs(:request_method).returns('DELETE')
             @mock_request.stubs(:path).returns('/foo')
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)            
         end
 
@@ -240,7 +248,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by destroy" do
             setup_destroy_request
             @mock_model_class.expects(:destroy).raises(ArgumentError) 
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)                 
         end       
     end
@@ -250,7 +258,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
             @mock_request.stubs(:request_method).returns('PUT')
             @mock_request.stubs(:path).returns('/foo')
             @mock_request.stubs(:body).returns('')
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)            
         end
 
@@ -279,7 +287,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by save" do
             setup_save_request
             @mock_model_instance.expects(:save).raises(ArgumentError) 
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)                         
         end
     end
@@ -310,14 +318,8 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
         it "should serialize a controller exception when an exception is thrown by search" do
             setup_search_request
             @mock_model_class.expects(:search).raises(ArgumentError) 
-            @mock_response.expects(:status=).with(404)
+            @mock_response.expects(:status=).with(400)
             @handler.process(@mock_request, @mock_response)                
         end
     end
-
-    it "should serialize a controller exception if the request fails" do
-        setup_bad_request     
-        @mock_response.expects(:status=).with(404)
-        @handler.process(@mock_request, @mock_response)        
-    end
 end