Seems a bit odd.

The code looks good, it just seems both a strange problem and a  
somewhat strange solution.

Should we always just treat them as IO objects?  What's the benefit of  
treating it like a string?

On Jul 4, 2009, at 1:50 PM, Jordan Curzon wrote:

>
> Mongrel::HttpRequest objects are sometimes a string, and sometimes
> they are a Tempfile or a StringIO. The Mongrel documentation  
> recommends
> regarding both as a file. The IO objects appear to be open and at the
> end, so we rewind them and read it into a string. This could be
> dangerous if the data is too large, but the API for receiving RESTful
> posts currently only accepts strings.
>
> Signed-off-by: Jordan Curzon <[email protected]>
> ---
> lib/puppet/network/http/mongrel/rest.rb |    9 ++++++++-
> spec/unit/network/http/mongrel/rest.rb  |    8 ++++++++
> 2 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/ 
> network/http/mongrel/rest.rb
> index 369d4c6..d1800ff 100644
> --- a/lib/puppet/network/http/mongrel/rest.rb
> +++ b/lib/puppet/network/http/mongrel/rest.rb
> @@ -37,7 +37,14 @@ class Puppet::Network::HTTP::MongrelREST <  
> Mongrel::HttpHandler
>
>     # return the request body
>     def body(request)
> -        request.body
> +        # See 
> http://mongrel.rubyforge.org/web/mongrel/classes/Mongrel/HttpRequest.html
> +        # for more information about #body and it's possible values
> +        if (body = request.body).is_a?(String)
> +            body
> +        else
> +            body.rewind
> +            body.read
> +        end
>     end
>
>     def set_content_type(response, format)
> diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/ 
> network/http/mongrel/rest.rb
> index abd573a..8dfff65 100755
> --- a/spec/unit/network/http/mongrel/rest.rb
> +++ b/spec/unit/network/http/mongrel/rest.rb
> @@ -58,6 +58,14 @@ describe "Puppet::Network::HTTP::MongrelREST" do
>                 @handler.body(@request).should == "mybody"
>             end
>
> +            it "should return the contents of IO-based request  
> bodies" do
> +                io = StringIO.new
> +                io.write 'mybody'
> +                @request.expects(:body).returns io
> +
> +                @handler.body(@request).should == "mybody"
> +            end
> +
>             it "should set the response's content-type header when  
> setting the content type" do
>                 @header = mock 'header'
>                 @response.expects(:header).returns @header
> -- 
> 1.6.3.3
>
>
> >


-- 
Some people are afraid of heights. I'm afraid of widths.
     -- Stephen Wright
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to