This commit allows streamed deserialization if the formats supports streaming.
Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/indirector/rest.rb | 14 +++++--- lib/puppet/network/response_stream.rb | 45 ++++++++++++++++++++++++ spec/unit/indirector/rest.rb | 45 +++++++++++++++---------- spec/unit/network/response_stream.rb | 60 +++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 23 deletions(-) create mode 100644 lib/puppet/network/response_stream.rb create mode 100644 spec/unit/network/response_stream.rb diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index a89e986..d3fb5ae 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -3,6 +3,7 @@ require 'uri' require 'puppet/network/http_pool' require 'puppet/network/http/api/v1' +require 'puppet/network/response_stream' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus @@ -44,10 +45,11 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus content_type = response['content-type'].gsub(/\s*;.*$/,'') # strip any appended charset # Convert the response to a deserialized object. + response = Puppet::Network::ResponseStream.new(response) if multiple - model.convert_from_multiple(content_type, response.body) + model.convert_from_multiple(content_type, response) else - model.convert_from(content_type, response.body) + model.convert_from(content_type, response) end else # Raise the http error if we didn't get a 'success' of some kind. @@ -66,14 +68,16 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end def find(request) - return nil unless result = deserialize(network(request).get(indirection2uri(request), headers)) + return nil unless result = network(request).request_get(indirection2uri(request), headers) do |response| + return deserialize(response) + end result.name = request.key result end def search(request) - unless result = deserialize(network(request).get(indirection2uri(request), headers), true) - return [] + return [] unless result = network(request).request_get(indirection2uri(request), headers) do |response| + return deserialize(response, true) end return result end diff --git a/lib/puppet/network/response_stream.rb b/lib/puppet/network/response_stream.rb new file mode 100644 index 0000000..4b7c772 --- /dev/null +++ b/lib/puppet/network/response_stream.rb @@ -0,0 +1,45 @@ + +require 'net/http' + +# This is a wrapper around either a Net::HTTPResponse or a String +# allowing the same interface for the exterior world. +class Puppet::Network::ResponseStream + + attr_accessor :response + + [:code, :body].each do |m| + define_method(m) do + response.send(:m) + end + end + + def initialize(content) + @response = content + end + + def stream? + response.is_a?(Net::HTTPResponse) + end + + def content + response.body + end + + def length + if stream? + response.content_length + else + response.length + end + end + + def stream + if stream? + response.read_body do |r| + yield r + end + else + yield response.body + end + end +end \ No newline at end of file diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index d12e3c6..fb6c03c 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -123,35 +123,42 @@ describe Puppet::Indirector::REST do } it "should return the results of converting from the format specified by the content-type header if the response code is in the 200s" do - @model.expects(:convert_from).with("myformat", "mydata").returns "myobject" - response = mock 'response' response.stubs(:[]).with("content-type").returns "myformat" response.stubs(:body).returns "mydata" response.stubs(:code).returns "200" + stream = stub 'stream' + Puppet::Network::ResponseStream.expects(:new).with(response).returns(stream) + + @model.expects(:convert_from).with("myformat", stream).returns "myobject" @searcher.deserialize(response).should == "myobject" end it "should convert and return multiple instances if the return code is in the 200s and 'multiple' is specified" do - @model.expects(:convert_from_multiple).with("myformat", "mydata").returns "myobjects" - response = mock 'response' response.stubs(:[]).with("content-type").returns "myformat" response.stubs(:body).returns "mydata" response.stubs(:code).returns "200" + stream = stub 'stream' + Puppet::Network::ResponseStream.expects(:new).with(response).returns(stream) + + @model.expects(:convert_from_multiple).with("myformat", stream).returns "myobjects" @searcher.deserialize(response, true).should == "myobjects" end it "should strip the content-type header to keep only the mime-type" do - @model.expects(:convert_from).with("text/plain", "mydata").returns "myobject" - response = mock 'response' response.stubs(:[]).with("content-type").returns "text/plain; charset=utf-8" response.stubs(:body).returns "mydata" response.stubs(:code).returns "200" + stream = stub 'stream' + Puppet::Network::ResponseStream.expects(:new).with(response).returns(stream) + + @model.expects(:convert_from).with("text/plain", stream).returns "myobject" + @searcher.deserialize(response) end end @@ -186,7 +193,8 @@ describe Puppet::Indirector::REST do describe "when doing a find" do before :each do - @connection = stub('mock http connection', :get => @response) + @connection = stub('mock http connection') + @connection.stubs(:request_get).yields(@response) @searcher.stubs(:network).returns(@connection) # neuter the network connection # Use a key with spaces, so we can test escaping @@ -195,27 +203,27 @@ describe Puppet::Indirector::REST do it "should call the GET http method on a network connection" do @searcher.expects(:network).returns @connection - @connection.expects(:get).returns @response + @connection.expects(:request_get).yields @response @searcher.find(@request) end it "should deserialize and return the http response" do - @connection.expects(:get).returns @response instance = stub 'object', :name= => nil @searcher.expects(:deserialize).with(@response).returns instance + @connection.expects(:request_get).yields(@response).returns(instance) @searcher.find(@request).should == instance end it "should use the URI generated by the Handler module" do @searcher.expects(:indirection2uri).with(@request).returns "/my/uri" - @connection.expects(:get).with { |path, args| path == "/my/uri" }.returns(@response) + @connection.expects(:request_get).with { |path, args| path == "/my/uri" }.yields(@response) @searcher.find(@request) end 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) + @connection.expects(:request_get).with { |path, args| args["Accept"] == "supported, formats" }.yields(@response) @searcher.model.expects(:supported_formats).returns %w{supported formats} @searcher.find(@request) @@ -227,7 +235,7 @@ describe Puppet::Indirector::REST do end it "should set the name of the resulting instance to the asked-for name" do - @searcher.expects(:deserialize).with(@response).returns @instance + @connection.expects(:request_get).returns @instance @instance.expects(:name=).with "foo bar" @searcher.find(@request) end @@ -240,7 +248,8 @@ describe Puppet::Indirector::REST do describe "when doing a search" do before :each do - @connection = stub('mock http connection', :get => @response) + @connection = stub('mock http connection') + @connection.stubs(:request_get).yields(@response) @searcher.stubs(:network).returns(@connection) # neuter the network connection @model.stubs(:convert_from_multiple) @@ -250,12 +259,12 @@ describe Puppet::Indirector::REST do it "should call the GET http method on a network connection" do @searcher.expects(:network).returns @connection - @connection.expects(:get).returns @response + @connection.expects(:request_get).yields @response @searcher.search(@request) end it "should deserialize as multiple instances and return the http response" do - @connection.expects(:get).returns @response + @connection.expects(:request_get).yields(@response).returns "myobject" @searcher.expects(:deserialize).with(@response, true).returns "myobject" @searcher.search(@request).should == 'myobject' @@ -263,19 +272,19 @@ describe Puppet::Indirector::REST do it "should use the URI generated by the Handler module" do @searcher.expects(:indirection2uri).with(@request).returns "/mys/uri" - @connection.expects(:get).with { |path, args| path == "/mys/uri" }.returns(@response) + @connection.expects(:request_get).with { |path, args| path == "/mys/uri" }.yields(@response) @searcher.search(@request) end 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) + @connection.expects(:request_get).with { |path, args| args["Accept"] == "supported, formats" }.yields(@response) @searcher.model.expects(:supported_formats).returns %w{supported formats} @searcher.search(@request) end it "should return an empty array if serialization returns nil" do - @model.stubs(:convert_from_multiple).returns nil + @connection.expects(:request_get).returns nil @searcher.search(@request).should == [] end diff --git a/spec/unit/network/response_stream.rb b/spec/unit/network/response_stream.rb new file mode 100644 index 0000000..08b23b5 --- /dev/null +++ b/spec/unit/network/response_stream.rb @@ -0,0 +1,60 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +require 'puppet/network/response_stream' + +describe Puppet::Network::ResponseStream do + before(:each) do + @content = stub_everything 'content' + @stream = Puppet::Network::ResponseStream.new(@content) + end + + it "should identify itself as a stream if the underlying content is an http response" do + @content.expects(:is_a?).with(Net::HTTPResponse).returns(true) + @stream.should be_stream + end + + it "should not identify itself as a stream if the underlying content is not an http response" do + @content.expects(:is_a?).with(Net::HTTPResponse).returns(false) + @stream.should_not be_stream + end + + it "should be able to return content length" do + @stream.should respond_to(:length) + end + + it "should be able to stream content" do + @stream.should respond_to(:stream) + end + + describe "when asking for content length" do + it "should return the content-length header if it is a stream" do + @stream.stubs(:stream?).returns(true) + @content.expects(:content_length).returns 10 + @stream.length.should == 10 + end + + it "should return the string length otherwise" do + @content.expects(:length).returns 10 + @stream.length.should == 10 + end + end + + describe "when streaming" do + it "should yield the block to the response read_body method if it is a stream" do + @stream.stubs(:stream?).returns(true) + @content.expects(:read_body).yields("chunk") + @stream.stream do |chunk| + chunk.should == "chunk" + end + end + + it "should yield the full body if it is not a stream" do + @content.expects(:body).returns("body") + @stream.stream do |chunk| + chunk.should == "body" + end + end + end +end \ No newline at end of file -- 1.6.6.1 -- 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.
