Fix #3373 - Client side file streaming

This patch moves file content writing to the content properties and
always write (or read) contents by chunks.

This reduces drastically puppetd memory consumption when handling large
sourced files.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

12 files changed, 399 insertions, 106 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 67bba5c0a..e94761503 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -719,35 +719,33 @@ Puppet::Type.newtype(:file) do
         obj
     end
 
-    # Write out the file.  Requires the content to be written,
-    # the property name for logging, and the checksum for validation.
-    def write(content, property, checksum = nil)
+    # Write out the file.  Requires the property name for logging.
+    # Write will be done by the content property, along with checksum computation
+    def write(property)
         remove_existing(:file)
 
-        use_temporary_file = (content.length != 0)
+        use_temporary_file = write_temporary_file?
         if use_temporary_file
             path = "#{self[:path]}.puppettmp_#{rand(10000)}"
             while File.exists?(path) or File.symlink?(path)
                 path = "#{self[:path]}.puppettmp_#{rand(10000)}"
-                end
-          else
+            end
+        else
             path = self[:path]
-          end
+        end
 
         mode = self.should(:mode) # might be nil
         umask = mode ? 000 : 022
 
-        Puppet::Util.withumask(umask) do
-            File.open(path, File::CREAT|File::WRONLY|File::TRUNC, mode) { |f| f.print content }
-        end
+        content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w', mode) { |f| write_content(f) } }
 
         # And put our new file in place
         if use_temporary_file # This is only not true when our file is empty.
             begin
-                fail_if_checksum_is_wrong(path, content) if validate_checksum?
+                fail_if_checksum_is_wrong(path, content_checksum) if validate_checksum?
                 File.rename(path, self[:path])
             rescue => detail
-                fail "Could not rename temporary file %s to %s : %s" % [path, self[:path], detail]
+                fail "Could not rename temporary file %s to %s: %s" % [path, self[:path], detail]
             ensure
                 # Make sure the created file gets removed
                 File.unlink(path) if FileTest.exists?(path)
@@ -761,19 +759,32 @@ Puppet::Type.newtype(:file) do
 
     private
 
+    # Should we validate the checksum of the file we're writing?
     def validate_checksum?
-        false
+         self[:checksum] !~ /time/
     end
 
     # Make sure the file we wrote out is what we think it is.
-    def fail_if_checksum_is_wrong(path, checksum)
-        # Use the appropriate checksum type -- md5, md5lite, etc.
-        checksum = parameter(:checksum).sum(content)
-
+    def fail_if_checksum_is_wrong(path, content_checksum)
         newsum = parameter(:checksum).sum_file(path)
-        return if [:absent, nil, checksum].include?(newsum)
+        return if [:absent, nil, content_checksum].include?(newsum)
+
+        self.fail "File written to disk did not match checksum; discarding changes (%s vs %s)" % [content_checksum, newsum]
+    end
+
+    # write the current content. Note that if there is no content property
+    # simply opening the file with 'w' as done in write is enough to truncate
+    # or write an empty length file.
+    def write_content(file)
+        (content = property(:content)) && content.write(file)
+    end
+
+    private
 
-        self.fail "File written to disk did not match checksum; discarding changes (%s vs %s)" % [checksum, newsum]
+    def write_temporary_file?
+        # unfortunately we don't know the source file size before fetching it
+        # so let's assume the file won't be empty 
+        (c = property(:content) and c.length) || (s = @parameters[:source] and 1)
     end
 
     # There are some cases where all of the work does not get done on
diff --git a/lib/puppet/type/file/checksum.rb b/lib/puppet/type/file/checksum.rb
index d4c24886c..3e2fdbf09 100755
--- a/lib/puppet/type/file/checksum.rb
+++ b/lib/puppet/type/file/checksum.rb
@@ -23,4 +23,11 @@ Puppet::Type.type(:file).newparam(:checksum) do
         method = type.to_s + "_file"
         "{#{type}}" + send(method, path).to_s
     end
+
+    def sum_stream(&block)
+        type = value || :md5 # same comment as above
+        method = type.to_s + "_stream"
+        checksum = send(method, &block)
+        "{#{type}}#{checksum}"
+    end
 end
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index cc2494b00..1817d5646 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -1,9 +1,16 @@
+require 'net/http'
+require 'uri'
+
 require 'puppet/util/checksums'
+require 'puppet/network/http/api/v1'
 
 module Puppet
     Puppet::Type.type(:file).newproperty(:content) do
         include Puppet::Util::Diff
         include Puppet::Util::Checksums
+        include Puppet::Network::HTTP::API::V1
+
+        attr_reader :actual_content
 
         desc "Specify the contents of a file as a string.  Newlines, tabs, and
             spaces can be specified using the escaped syntax (e.g., \\n for a
@@ -68,21 +75,12 @@ module Puppet
             end
         end
 
-        # If content was specified, return that; else try to return the source content;
-        # else, return nil.
-        def actual_content
-            if defined?(@actual_content) and @actual_content
-                return @actual_content
-            end
-
-            if s = resource.parameter(:source)
-                return s.content
-            end
-            fail "Could not find actual content from checksum"
+        def length
+            (actual_content and actual_content.length) || 0
         end
 
         def content
-            self.should || (s = resource.parameter(:source) and s.content)
+            self.should
         end
 
         # Override this method to provide diffs if asked for.
@@ -132,9 +130,46 @@ module Puppet
             # We're safe not testing for the 'source' if there's no 'should'
             # because we wouldn't have gotten this far if there weren't at least
             # one valid value somewhere.
-            @resource.write(actual_content, :content)
+            @resource.write(:content)
 
             return return_event
         end
+
+        def write(file)
+            self.fail "Writing content that wasn't provided" unless actual_content || resource.parameter(:source)
+            resource.parameter(:checksum).sum_stream { |sum|
+                each_chunk_from(actual_content || resource.parameter(:source)) { |chunk|
+                    sum << chunk
+                    file.print chunk
+                }
+            }
+        end
+
+        def each_chunk_from(source_or_content)
+            if source_or_content.is_a?(String)
+                yield source_or_content
+            elsif source_or_content.nil?
+                nil
+            elsif source_or_content.local?
+                File.open(source_or_content.full_path, "r") do |src|
+                    while chunk = src.read(8192)
+                        yield chunk
+                    end
+                end
+            else
+                request = Puppet::Indirector::Request.new(:file_content, :find, source_or_content.full_path)
+                connection = Puppet::Network::HttpPool.http_instance(source_or_content.server, source_or_content.port)
+                connection.request_get(indirection2uri(request), {"Accept" => "raw"}) do |response|
+                    case response.code
+                    when "404"; nil
+                    when /^2/;  response.read_body { |chunk| yield chunk }
+                    else
+                        # Raise the http error if we didn't get a 'success' of some kind.
+                        message = "Error %s on SERVER: %s" % [response.code, (response.body||'').empty? ? response.message : response.body]
+                        raise Net::HTTPError.new(message, response)
+                    end
+                end
+            end
+        end
     end
 end
diff --git a/lib/puppet/type/file/ensure.rb b/lib/puppet/type/file/ensure.rb
index b2dd39459..83f3d3e6a 100755
--- a/lib/puppet/type/file/ensure.rb
+++ b/lib/puppet/type/file/ensure.rb
@@ -45,7 +45,7 @@ module Puppet
             if property = @resource.property(:content)
                 property.sync
             else
-                @resource.write("", :ensure)
+                @resource.write(:ensure)
                 mode = @resource.should(:mode)
             end
         end
diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb
index b3d9b3ec2..1eb418e90 100755
--- a/lib/puppet/type/file/source.rb
+++ b/lib/puppet/type/file/source.rb
@@ -96,16 +96,6 @@ module Puppet
             metadata && metadata.checksum
         end
 
-        # Look up (if necessary) and return remote content.
-        cached_attr(:content) do
-            raise Puppet::DevError, "No source for content was stored with the metadata" unless metadata.source
-
-            unless tmp = Puppet::FileServing::Content.find(metadata.source)
-                fail "Could not find any content at %s" % metadata.source
-            end
-            tmp.content
-        end
-
         # Copy the values from the source to the resource.  Yay.
         def copy_source_values
             devfail "Somehow got asked to copy source values without any metadata" unless metadata
@@ -175,5 +165,29 @@ module Puppet
             resource[:check] = checks
             resource[:checksum] = :md5 unless resource.property(:checksum)
         end
+
+        def local?
+            found? and uri and (uri.scheme || "file") == "file"
+        end
+
+        def full_path
+            if found? and uri
+                return URI.unescape(uri.path)
+            end
+        end
+
+        def server
+            (uri and uri.host) or Puppet.settings[:server]
+        end
+
+        def port
+            (uri and uri.port) or Puppet.settings[:masterport]
+        end
+
+        private
+
+        def uri
+            @uri ||= URI.parse(URI.escape(metadata.source))
+        end
     end
 end
diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
index f68f77624..e534fb0fb 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -39,11 +39,27 @@ module Puppet::Util::Checksums
         md5_file(filename, true)
     end
 
+    def md5_stream(&block)
+        require 'digest/md5'
+        digest = Digest::MD5.new()
+        yield digest
+        return digest.hexdigest
+    end
+
+    alias :md5lite_stream :md5_stream
+
     # Return the :mtime timestamp of a file.
     def mtime_file(filename)
         File.stat(filename).send(:mtime)
     end
 
+    # by definition this doesn't exist
+    def mtime_stream
+        nil
+    end
+
+    alias :ctime_stream :mtime_stream
+
     # Calculate a checksum using Digest::SHA1.
     def sha1(content)
         require 'digest/sha1'
@@ -68,6 +84,15 @@ module Puppet::Util::Checksums
         sha1_file(filename, true)
     end
 
+    def sha1_stream
+        require 'digest/sha1'
+        digest = Digest::SHA1.new()
+        yield digest
+        return digest.hexdigest
+    end
+
+    alias :sha1lite_stream :sha1_stream
+
     # Return the :ctime of a file.
     def ctime_file(filename)
         File.stat(filename).send(:ctime)
diff --git a/spec/integration/type/file.rb b/spec/integration/type/file.rb
index 6222f0a89..0724467cb 100755
--- a/spec/integration/type/file.rb
+++ b/spec/integration/type/file.rb
@@ -134,7 +134,7 @@ describe Puppet::Type.type(:file) do
 
             File.expects(:rename).raises ArgumentError
 
-            lambda { file.write("something", :content) }.should raise_error(Puppet::Error)
+            lambda { file.write(:content) }.should raise_error(Puppet::Error)
             File.read(file[:path]).should == "bar"
         end
     end
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index f9c691b11..4213753a4 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -43,18 +43,39 @@ describe Puppet::Type.type(:file) do
             File.expects(:rename).raises ArgumentError
             file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
 
-            lambda { file.write("something", :content) }.should raise_error(Puppet::Error)
+            file.stubs(:validate_checksum?).returns(false)
+
+            property = stub('content_property', :actual_content => "something", :length => "something".length)
+            file.stubs(:property).with(:content).returns(property)
+
+            lambda { file.write(:content) }.should raise_error(Puppet::Error)
+        end
+
+        it "should delegate writing to the content property" do
+            filehandle = stub_everything 'fh'
+            File.stubs(:open).yields(filehandle)
+            File.stubs(:rename)
+            property = stub('content_property', :actual_content => "something", :length => "something".length)
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
+            file.stubs(:validate_checksum?).returns(false)
+            file.stubs(:property).with(:content).returns(property)
+
+            property.expects(:write).with(filehandle)
+
+            file.write(:content)
         end
 
         describe "when validating the checksum" do
             before { @file.stubs(:validate_checksum?).returns(true) }
 
-            it "should fail if the checksum property and content checksums do not match" do
-                property = stub('checksum_property', :checktype => :md5, :md5 => 'checksum_a', :getsum => 'checksum_b')
-                @file.stubs(:property).with(:checksum).returns(property)
+            it "should fail if the checksum parameter and content checksums do not match" do
+                checksum = stub('checksum_parameter',  :sum => 'checksum_b')
+                @file.stubs(:parameter).with(:checksum).returns(checksum)
 
-                @file.stubs(:validate_checksum?).returns(true)
-                lambda { @file.write "something", :NOTUSED }.should raise_error(Puppet::Error)
+                property = stub('content_property', :actual_content => "something", :length => "something".length, :write => 'checksum_a')
+                @file.stubs(:property).with(:content).returns(property)
+
+                lambda { @file.write :NOTUSED }.should raise_error(Puppet::Error)
             end
         end
 
@@ -62,10 +83,13 @@ describe Puppet::Type.type(:file) do
             before { @file.stubs(:validate_checksum?).returns(false) }
 
             it "should not fail if the checksum property and content checksums do not match" do
-                property = stub('checksum_property', :checktype => :md5, :md5 => 'checksum_a', :getsum => 'checksum_b')
-                @file.stubs(:property).with(:checksum).returns(property)
+                checksum = stub('checksum_parameter',  :sum => 'checksum_b')
+                @file.stubs(:parameter).with(:checksum).returns(checksum)
+
+                property = stub('content_property', :actual_content => "something", :length => "something".length, :write => 'checksum_a')
+                @file.stubs(:property).with(:content).returns(property)
 
-                lambda { @file.write "something", :NOTUSED }.should_not raise_error(Puppet::Error)
+                lambda { @file.write :NOTUSED }.should_not raise_error(Puppet::Error)
             end
 
         end
@@ -840,17 +864,6 @@ describe Puppet::Type.type(:file) do
         end
     end
 
-    describe "when writing the file" do
-        it "should propagate failures encountered when renaming the temporary file" do
-            File.stubs(:open)
-
-            File.expects(:rename).raises ArgumentError
-            file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
-
-            lambda { file.write("something", :content) }.should raise_error(Puppet::Error)
-        end
-    end
-
     describe "when retrieving the current file state" do
         it "should copy the source values if the 'source' parameter is set" do
             file = Puppet::Type::File.new(:name => "/my/file", :source => "/foo/bar")
diff --git a/spec/unit/type/file/checksum.rb b/spec/unit/type/file/checksum.rb
index 8f0f84290..e6a853042 100644
--- a/spec/unit/type/file/checksum.rb
+++ b/spec/unit/type/file/checksum.rb
@@ -56,4 +56,18 @@ describe checksum do
         @checksum.expects(:md5_file).returns "mysum"
         @checksum.sum_file("/foo/bar").should == "{md5}mysum"
     end
+
+    it "should return the summed contents of a stream with a checksum label" do
+        @resource[:checksum] = :md5
+        @checksum.expects(:md5_stream).returns "mysum"
+        @checksum.sum_stream.should == "{md5}mysum"
+    end
+
+    it "should yield the sum_stream block to the underlying checksum" do
+        @resource[:checksum] = :md5
+        @checksum.expects(:md5_stream).yields("something").returns("mysum")
+        @checksum.sum_stream do |sum|
+            sum.should == "something"
+        end
+    end
 end
diff --git a/spec/unit/type/file/content.rb b/spec/unit/type/file/content.rb
index 5a3a8598d..96f037f7d 100755
--- a/spec/unit/type/file/content.rb
+++ b/spec/unit/type/file/content.rb
@@ -44,19 +44,13 @@ describe content do
             @content.actual_content.should == "ehness"
         end
 
-        it "should use the content from the source if the source is set" do
+        it "should not use the content from the source if the source is set" do
             source = mock 'source'
-            source.expects(:content).returns "scont"
 
-            @resource.expects(:parameter).with(:source).returns source
+            @resource.expects(:parameter).never.with(:source).returns source
 
             @content = content.new(:resource => @resource)
-            @content.actual_content.should == "scont"
-        end
-
-        it "should fail if no source is available and no content is set" do
-            @content = content.new(:resource => @resource)
-            lambda { @content.actual_content }.should raise_error(Puppet::Error)
+            @content.actual_content.should be_nil
         end
     end
 
@@ -236,7 +230,7 @@ describe content do
         end
 
         it "should use the file's :write method to write the content" do
-            @resource.expects(:write).with("some content", :content)
+            @resource.expects(:write).with(:content)
 
             @content.sync
         end
@@ -253,4 +247,140 @@ describe content do
             @content.sync.should == :file_created
         end
     end
+
+    describe "when writing" do
+        before do
+            @content = content.new(:resource => @resource)
+            @fh = stub_everything
+        end
+
+        it "should fail if no actual content nor source exists" do
+            lambda { @content.write(@fh) }.should raise_error
+        end
+
+        describe "from actual content" do
+            before(:each) do
+                @content.stubs(:actual_content).returns("this is content")
+            end
+
+            it "should write to the given file handle" do
+                @fh.expects(:print).with("this is content")
+                @content.write(@fh)
+            end
+
+            it "should return the current checksum value" do
+                @resource.parameter(:checksum).expects(:sum_stream).returns "checksum"
+                @content.write(@fh).should == "checksum"
+            end
+        end
+
+        describe "from local source" do
+            before(:each) do
+                @content.stubs(:actual_content).returns(nil)
+                @source = stub_everything 'source', :local? => true, :full_path => "/path/to/source"
+                @resource.stubs(:parameter).with(:source).returns @source
+
+                @sum = stub_everything 'sum'
+                @resource.stubs(:parameter).with(:checksum).returns(@sum)
+
+                @digest = stub_everything 'digest'
+                @sum.stubs(:sum_stream).yields(@digest)
+
+                @file = stub_everything 'file'
+                File.stubs(:open).yields(@file)
+                @file.stubs(:read).with(8192).returns("chunk1").then.returns("chunk2").then.returns(nil)
+            end
+
+            it "should open the local file" do
+                File.expects(:open).with("/path/to/source", "r")
+                @content.write(@fh)
+            end
+
+            it "should read the local file by chunks" do
+                @file.expects(:read).with(8192).returns("chunk1").then.returns(nil)
+                @content.write(@fh)
+            end
+
+            it "should write each chunk to the file" do
+                @fh.expects(:print).with("chunk1").then.with("chunk2")
+                @content.write(@fh)
+            end
+
+            it "should pass each chunk to the current sum stream" do
+                @digest.expects(:<<).with("chunk1").then.with("chunk2")
+                @content.write(@fh)
+            end
+
+            it "should return the checksum computed" do
+                @sum.stubs(:sum_stream).yields(@digest).returns("checksum")
+                @content.write(@fh).should == "checksum"
+            end
+        end
+
+        describe "from remote source" do
+            before(:each) do
+                @response = stub_everything 'mock response', :code => "404"
+                @conn = stub_everything 'connection'
+                @conn.stubs(:request_get).yields(@response)
+                Puppet::Network::HttpPool.stubs(:http_instance).returns @conn
+
+                @content.stubs(:actual_content).returns(nil)
+                @source = stub_everything 'source', :local? => false, :full_path => "/path/to/source", :server => "server", :port => 1234
+                @resource.stubs(:parameter).with(:source).returns @source
+
+                @sum = stub_everything 'sum'
+                @resource.stubs(:parameter).with(:checksum).returns(@sum)
+
+                @digest = stub_everything 'digest'
+                @sum.stubs(:sum_stream).yields(@digest)
+            end
+
+            it "should open a network connection to source server and port" do
+                Puppet::Network::HttpPool.expects(:http_instance).with("server", 1234).returns @conn
+                @content.write(@fh)
+            end
+
+            it "should send the correct indirection uri" do
+                @conn.expects(:request_get).with { |uri,headers| uri == "/production/file_content//path/to/source" }.yields(@response)
+                @content.write(@fh)
+            end
+
+            it "should return nil if source is not found" do
+                @response.expects(:code).returns("404")
+                @content.write(@fh).should == nil
+            end
+
+            it "should not write anything if source is not found" do
+                @response.expects(:code).returns("404")
+                @fh.expects(:print).never
+                @content.write(@fh).should == nil
+            end
+
+            it "should raise an HTTP error in case of server error" do
+                @response.expects(:code).returns("500")
+                lambda { @content.write(@fh) }.should raise_error
+            end
+
+            it "should write content by chunks" do
+                @response.expects(:code).returns("200")
+                @response.expects(:read_body).multiple_yields("chunk1","chunk2")
+                @fh.expects(:print).with("chunk1").then.with("chunk2")
+                @content.write(@fh)
+            end
+
+            it "should pass each chunk to the current sum stream" do
+                @response.expects(:code).returns("200")
+                @response.expects(:read_body).multiple_yields("chunk1","chunk2")
+                @digest.expects(:<<).with("chunk1").then.with("chunk2")
+                @content.write(@fh)
+            end
+
+            it "should return the checksum computed" do
+                @response.expects(:code).returns("200")
+                @response.expects(:read_body).multiple_yields("chunk1","chunk2")
+                @sum.expects(:sum_stream).yields(@digest).returns("checksum")
+                @content.write(@fh).should == "checksum"
+            end
+        end
+    end
 end
diff --git a/spec/unit/type/file/source.rb b/spec/unit/type/file/source.rb
index e4e9ad9d1..b9bb22240 100755
--- a/spec/unit/type/file/source.rb
+++ b/spec/unit/type/file/source.rb
@@ -198,51 +198,75 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         end
     end
 
-    it "should have a method for returning the content" do
-        source.new(:resource => @resource).must respond_to(:content)
+    it "should have a local? method" do
+        source.new(:resource => @resource).must be_respond_to(:local?)
     end
 
-    describe "when looking up the content" do
-        before do
+    context "when accessing source properties" do
+        before(:each) do
             @source = source.new(:resource => @resource)
-            @metadata = stub 'metadata', :source => "/my/source"
-            @source.stubs(:metadata).returns @metadata
-
-            @content = stub 'content', :content => "foobar"
+            @metadata = stub_everything
+            @source.stubs(:metadata).returns(@metadata)
         end
 
-        it "should fail if the metadata does not have a source set" do
-            @metadata.stubs(:source).returns nil
-            lambda { @source.content }.should raise_error(Puppet::DevError)
-        end
+        describe "for local sources" do
+            before(:each) do
+                @metadata.stubs(:ftype).returns "file"
+                @metadata.stubs(:source).returns("file:///path/to/source")
+            end
 
-        it "should look the content up from the Content class using the metadata source if no content is set" do
-            Puppet::FileServing::Content.expects(:find).with("/my/source").returns @content
-            @source.content.should == "foobar"
-        end
+            it "should be local" do
+                @source.must be_local
+            end
 
-        it "should return previously found content" do
-            Puppet::FileServing::Content.expects(:find).with("/my/source").returns @content
-            @source.content.should == "foobar"
-            @source.content.should == "foobar"
-        end
+            it "should be local if there is no scheme" do
+                @metadata.stubs(:source).returns("/path/to/source")
+                @source.must be_local
+            end
 
-        it "should fail if no content can be retrieved" do
-            Puppet::FileServing::Content.expects(:find).with("/my/source").returns nil
-            @source.expects(:fail).raises RuntimeError
-            lambda { @source.content }.should raise_error(RuntimeError)
+            it "should be able to return the metadata source full path" do
+                @source.full_path.should == "/path/to/source"
+            end
         end
 
-        it "should expire the content appropriately" do
-            expirer = stub 'expired', :dependent_data_expired? => true
+        describe "for remote sources" do
+            before(:each) do
+                @metadata.stubs(:ftype).returns "file"
+                @metadata.stubs(:source).returns("puppet://server:8192/path/to/source")
+            end
 
-            content2 = stub 'content', :content => "secondrun"
-            Puppet::FileServing::Content.expects(:find).with("/my/source").times(2).returns(@content).then.returns(content2)
-            @source.content.should == "foobar"
+            it "should not be local" do
+                @source.should_not be_local
+            end
 
-            @source.stubs(:expirer).returns expirer
+            it "should be able to return the metadata source full path" do
+                @source.full_path.should == "/path/to/source"
+            end
+
+            it "should be able to return the source server" do
+                @source.server.should == "server"
+            end
+
+            it "should be able to return the source port" do
+                @source.port.should == 8192
+            end
 
-            @source.content.should == "secondrun"
+            describe "which don't specify server or port" do
+                before(:each) do
+                    @metadata.stubs(:source).returns("puppet:///path/to/source")
+                end
+
+                it "should return the default source server" do
+                    Puppet.settings.expects(:[]).with(:server).returns("myserver")
+                    @source.server.should == "myserver"
+                end
+
+                it "should return the default source port" do
+                    Puppet.settings.expects(:[]).with(:masterport).returns(1234)
+                    @source.port.should == 1234
+                end
+            end
         end
     end
+
 end
diff --git a/spec/unit/util/checksums.rb b/spec/unit/util/checksums.rb
index e0c990962..eba564352 100755
--- a/spec/unit/util/checksums.rb
+++ b/spec/unit/util/checksums.rb
@@ -28,6 +28,12 @@ describe Puppet::Util::Checksums do
         end
     end
 
+    [content_sums, file_only].flatten.each do |sumtype|
+        it "should be able to calculate %s sums from stream" % sumtype do
+            @summer.should be_respond_to(sumtype.to_s + "_stream")
+        end
+    end
+
     it "should have a method for determining whether a given string is a checksum" do
         @summer.should respond_to(:checksum?)
     end
@@ -78,6 +84,16 @@ describe Puppet::Util::Checksums do
 
                 @summer.send(sum.to_s + "_file", file).should == :mydigest
             end
+
+            it "should yield #{klass} to the given block to calculate stream checksums" do
+                digest = mock 'digest'
+                klass.expects(:new).returns digest
+                digest.expects(:hexdigest).returns :mydigest
+
+                @summer.send(sum.to_s + "_stream") do |sum|
+                    sum.should == digest
+                end.should == :mydigest
+            end
         end
     end
 
@@ -118,6 +134,10 @@ describe Puppet::Util::Checksums do
 
                 @summer.send(sum.to_s + "_file", file).should == "mysum"
             end
+
+            it "should return nil for streams" do
+                @summer.send(sum.to_s + "_stream").should be_nil
+            end
         end
     end