On Jul 4, 2:52 pm, Brice Figureau <[email protected]>
wrote:
> 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
I like the idea of using proper mime types in the Content-Type
headers, Hopefully we can get the server responses to send proper mime
types too.
>
> 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
> +
I'm not sure you should use format_to_use for this purpose because it
is used to determine what format to use for the response. If a client
(or a 3rd party app) sends an Accepts header different than the
Content-Type header and both are suitable?, the client will receive
content that they didn't say they can accept. I think it should be a
parallel method to determine the format to use to decode request
bodies. format_to_use should look at the accept header and the other
method should look at the Content-Type header.
> 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)
> �[email protected](: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")
> �[email protected] = @instance
> end
> @@ -350,6 +350,13 @@ describe Puppet::Indirector::REST do
> �[email protected](@request)
> end
>
> + it "should provide a Content-Type header containing the mime-type of
> the sent object" do
> + �[email protected](:put).with { |path, data, args|
> args['Content-Type'] == "mime" }.returns(@response)
> +
> + �[email protected](:mime).returns "mime"
> + �[email protected](@request)
> + end
> +
> it "should deserialize and return the network response" do
> �[email protected](:deserialize).with(@response).returns
> @instance
> �[email protected](@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
> - �[email protected](:accept_header ).returns
> "format_one,format_two"
> - �[email protected](:set_content_type).returns "my_result"
> - �[email protected](:set_response ).returns "my_result"
> - �[email protected](:path ).returns
> "/my_handler/my_result"
> - �[email protected](:http_method ).returns("GET")
> - �[email protected](:params ).returns({})
> - �[email protected](:content_type
> ...
>
> read more »
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
