The way the format was given to the server was kind of broken when we were saving to the server. It was using the first member of the Accept header. The Accept header is usually reserved for the other side, ie what the client will accept when the server will respond.
This patch adds a Content-Type header containing the mime-type of the object sent by the client when saving. Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/indirector/rest.rb | 2 +- lib/puppet/network/format_handler.rb | 8 +++++++- lib/puppet/network/http/handler.rb | 17 ++++++++++++++--- lib/puppet/network/http/mongrel/rest.rb | 4 ++++ lib/puppet/network/http/rack/rest.rb | 5 +++++ lib/puppet/network/http/webrick/rest.rb | 4 ++++ spec/unit/indirector/rest.rb | 9 ++++++++- spec/unit/network/format_handler.rb | 26 ++++++++++++++++++++++++++ spec/unit/network/http/handler.rb | 26 ++++++++++++++++++-------- spec/unit/network/http/mongrel/rest.rb | 5 +++++ spec/unit/network/http/rack/rest.rb | 5 +++++ spec/unit/network/http/webrick/rest.rb | 5 +++++ 12 files changed, 102 insertions(+), 14 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 6903f9a..909be9a 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -81,7 +81,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus def save(request) raise ArgumentError, "PUT does not accept options" unless request.options.empty? - deserialize network(request).put(indirection2uri(request), request.instance.render, headers) + deserialize network(request).put(indirection2uri(request), request.instance.render, headers.merge({ "Content-Type" => request.instance.mime })) end private diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb index 0a7e9dc..839fe66 100644 --- a/lib/puppet/network/format_handler.rb +++ b/lib/puppet/network/format_handler.rb @@ -23,7 +23,7 @@ module Puppet::Network::FormatHandler @format = format end - [:intern, :intern_multiple, :render, :render_multiple].each do |method| + [:intern, :intern_multiple, :render, :render_multiple, :mime].each do |method| define_method(method) do |*args| protect(method, args) end @@ -123,6 +123,12 @@ module Puppet::Network::FormatHandler Puppet::Network::FormatHandler.protected_format(format).render(self) end + def mime(format = nil) + format ||= self.class.default_format + + Puppet::Network::FormatHandler.protected_format(format).mime + end + def support_format?(name) self.class.support_format?(name) end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index c6b809d..0383713 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -17,9 +17,20 @@ module Puppet::Network::HTTP::Handler raise NotImplementedError end - # Which format to use when serializing our response. Just picks - # the first value in the accept header, at this point. + # Retrieve the Content-Type header from the http request. + def content_type_header(request) + raise NotImplementedError + end + + # Which format to use when serializing our response or interpreting the request. + # IF the client provided a Content-Type use this, otherwise use the Accept header + # and just pick the first value. def format_to_use(request) + if header = content_type_header(request) + format = Puppet::Network::FormatHandler.mime(header) + return format.name.to_s if format.suitable? + end + unless header = accept_header(request) raise ArgumentError, "An Accept header must be provided to pick the right format" end @@ -118,7 +129,7 @@ module Puppet::Network::HTTP::Handler format = format_to_use(request) - obj = indirection_request.model.convert_from(format_to_use(request), data) + obj = indirection_request.model.convert_from(format, data) result = save_object(indirection_request, obj) set_content_type(response, "yaml") diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index 369d4c6..6b1fafb 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -15,6 +15,10 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler request.params[ACCEPT_HEADER] end + def content_type_header(request) + request.params[Mongrel::Const::CONTENT_TYPE] + end + # which HTTP verb was used in this request def http_method(request) request.params[Mongrel::Const::REQUEST_METHOD] diff --git a/lib/puppet/network/http/rack/rest.rb b/lib/puppet/network/http/rack/rest.rb index e98bffc..a4d475d 100644 --- a/lib/puppet/network/http/rack/rest.rb +++ b/lib/puppet/network/http/rack/rest.rb @@ -28,6 +28,11 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler request.env[HEADER_ACCEPT] end + # Retrieve the accept header from the http request. + def content_type_header(request) + request.env['HTTP_CONTENT_TYPE'] + end + # Return which HTTP verb was used in this request. def http_method(request) request.request_method diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb index 5f77da8..b27b304 100644 --- a/lib/puppet/network/http/webrick/rest.rb +++ b/lib/puppet/network/http/webrick/rest.rb @@ -27,6 +27,10 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet request["accept"] end + def content_type_header(request) + request["content-type"] + end + def http_method(request) request.request_method end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 9f77deb..696af9a 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -306,7 +306,7 @@ describe Puppet::Indirector::REST do @connection = stub('mock http connection', :put => @response) @searcher.stubs(:network).returns(@connection) # neuter the network connection - @instance = stub 'instance', :render => "mydata" + @instance = stub 'instance', :render => "mydata", :mime => "mime" @request = Puppet::Indirector::Request.new(:foo, :save, "foo bar") @request.instance = @instance end @@ -350,6 +350,13 @@ describe Puppet::Indirector::REST do @searcher.save(@request) end + it "should provide a Content-Type header containing the mime-type of the sent object" do + @connection.expects(:put).with { |path, data, args| args['Content-Type'] == "mime" }.returns(@response) + + @instance.expects(:mime).returns "mime" + @searcher.save(@request) + end + it "should deserialize and return the network response" do @searcher.expects(:deserialize).with(@response).returns @instance @searcher.save(@request).should equal(@instance) diff --git a/spec/unit/network/format_handler.rb b/spec/unit/network/format_handler.rb index 54409b9..74ee4f5 100755 --- a/spec/unit/network/format_handler.rb +++ b/spec/unit/network/format_handler.rb @@ -202,6 +202,10 @@ describe Puppet::Network::FormatHandler do FormatTester.new.should respond_to(:render) end + it "should be able to get a format mime-type" do + FormatTester.new.should respond_to(:mime) + end + it "should raise a FormatError when a rendering error is encountered" do format = stub 'rendering format', :supported? => true Puppet::Network::FormatHandler.stubs(:format).with(:foo).returns format @@ -233,5 +237,27 @@ describe Puppet::Network::FormatHandler do format.expects(:render).with(tester) tester.render end + + it "should call the format-specific converter when asked for the mime-type of a given format" do + format = stub 'rendering format', :supported? => true + + Puppet::Network::FormatHandler.stubs(:format).with(:foo).returns format + + tester = FormatTester.new + format.expects(:mime).returns "foo" + + tester.mime(:foo).should == "foo" + end + + it "should return the default format mime-type if no format is provided" do + format = stub 'rendering format', :supported? => true + Puppet::Network::FormatHandler.stubs(:format).with("foo").returns format + + FormatTester.expects(:default_format).returns "foo" + tester = FormatTester.new + + format.expects(:mime).returns "foo" + tester.mime.should == "foo" + end end end diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index 8ef6e00..97ad2d2 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -57,13 +57,14 @@ describe Puppet::Network::HTTP::Handler do # Stub out the interface we require our including classes to # implement. def stub_server_interface - @handler.stubs(:accept_header ).returns "format_one,format_two" - @handler.stubs(:set_content_type).returns "my_result" - @handler.stubs(:set_response ).returns "my_result" - @handler.stubs(:path ).returns "/my_handler/my_result" - @handler.stubs(:http_method ).returns("GET") - @handler.stubs(:params ).returns({}) - @handler.stubs(:content_type ).returns("text/plain") + @handler.stubs(:accept_header ).returns "format_one,format_two" + @handler.stubs(:content_type_header).returns nil + @handler.stubs(:set_content_type ).returns "my_result" + @handler.stubs(:set_response ).returns "my_result" + @handler.stubs(:path ).returns "/my_handler/my_result" + @handler.stubs(:http_method ).returns("GET") + @handler.stubs(:params ).returns({}) + @handler.stubs(:content_type ).returns("text/plain") end it "should create an indirection request from the path, parameters, and http method" do @@ -347,7 +348,7 @@ describe Puppet::Network::HTTP::Handler do @model_instance = stub('indirected model instance', :save => true) @model_class.stubs(:convert_from).returns(@model_instance) - @format = stub 'format', :suitable? => true + @format = stub 'format', :suitable? => true, :name => "format" Puppet::Network::FormatHandler.stubs(:format).returns @format end @@ -393,6 +394,15 @@ describe Puppet::Network::HTTP::Handler do @handler.expects(:set_content_type).with(@response, "yaml") @handler.do_save(@irequest, @request, @response) end + + it "should use the content-type header to know the body format" do + @handler.expects(:content_type_header).returns("text/format") + Puppet::Network::FormatHandler.stubs(:mime).with("text/format").returns @format + + @model_class.expects(:convert_from).with { |format, body| format == "format" }.returns @model_instance + + @handler.do_save(@irequest, @request, @response) + end end end diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb index abd573a..594dc44 100755 --- a/spec/unit/network/http/mongrel/rest.rb +++ b/spec/unit/network/http/mongrel/rest.rb @@ -43,6 +43,11 @@ describe "Puppet::Network::HTTP::MongrelREST" do @handler.accept_header(@request).should == "myaccept" end + it "should return the Content-Type parameter as the Content-Type header" do + @params.expects(:[]).with(Mongrel::Const::CONTENT_TYPE).returns "mycontent" + @handler.content_type_header(@request).should == "mycontent" + end + it "should use the REQUEST_METHOD as the http method" do @params.expects(:[]).with(Mongrel::Const::REQUEST_METHOD).returns "mymethod" @handler.http_method(@request).should == "mymethod" diff --git a/spec/unit/network/http/rack/rest.rb b/spec/unit/network/http/rack/rest.rb index 1e3efe9..3a36a0b 100755 --- a/spec/unit/network/http/rack/rest.rb +++ b/spec/unit/network/http/rack/rest.rb @@ -40,6 +40,11 @@ describe "Puppet::Network::HTTP::RackREST" do @handler.accept_header(req).should == "myaccept" end + it "should return the HTTP_CONTENT_TYPE parameter as the content type header" do + req = mk_req('/', 'HTTP_CONTENT_TYPE' => 'mycontent') + @handler.content_type_header(req).should == "mycontent" + end + it "should use the REQUEST_METHOD as the http method" do req = mk_req('/', :method => 'mymethod') @handler.http_method(req).should == "mymethod" diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb index 9d9f200..5509905 100755 --- a/spec/unit/network/http/webrick/rest.rb +++ b/spec/unit/network/http/webrick/rest.rb @@ -37,6 +37,11 @@ describe Puppet::Network::HTTP::WEBrickREST do @handler.accept_header(@request).should == "foobar" end + it "should use the 'content-type' request header as the Content-Type header" do + @request.expects(:[]).with("content-type").returns "foobar" + @handler.content_type_header(@request).should == "foobar" + end + it "should use the request method as the http method" do @request.expects(:request_method).returns "FOO" @handler.http_method(@request).should == "FOO" -- 1.6.0.2 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
