Fixing #1541 - ParsedFile only backs up files once per transaction

This moves responsibility for backups from the filetype
to the consumer of the filetype, but only ParsedFile actually uses
filetypes.

Signed-off-by: Luke Kanies <luke@madstop.com>

4 files changed, 50 insertions, 7 deletions

diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index a4c4bc87c..45eae57ff 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -78,8 +78,22 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         @modified.reject! { |t| flushed.include?(t) }
     end
 
+    # Make sure our file is backed up, but only back it up once per transaction.
+    # We cheat and rely on the fact that @records is created on each prefetch.
+    def self.backup_target(target)
+        unless defined?(@backup_stats)
+            @backup_stats = {}
+        end
+        return nil if @backup_stats[target] == @records.object_id
+
+        target_object(target).backup
+        @backup_stats[target] = @records.object_id
+    end
+
     # Flush all of the records relating to a specific target.
     def self.flush_target(target)
+        backup_target(target)
+
         records = target_records(target).reject { |r|
             r[:ensure] == :absent
         }
diff --git a/lib/puppet/util/filetype.rb b/lib/puppet/util/filetype.rb
index 137ef0c67..5d4ba1440 100755
--- a/lib/puppet/util/filetype.rb
+++ b/lib/puppet/util/filetype.rb
@@ -108,7 +108,6 @@ class Puppet::Util::FileType
 
         # Overwrite the file.
         def write(text)
-            backup()
             require "tempfile"
             tf = Tempfile.new("puppet") 
             tf.print text; tf.flush 
diff --git a/spec/unit/provider/parsedfile.rb b/spec/unit/provider/parsedfile.rb
index 05e9de3ab..11a91c8d7 100755
--- a/spec/unit/provider/parsedfile.rb
+++ b/spec/unit/provider/parsedfile.rb
@@ -47,4 +47,40 @@ describe Puppet::Provider::ParsedFile do
             @class.instances
         end
     end
+
+    describe "when flushing a file's records to disk" do
+        before do
+            # This way we start with some @records, like we would in real life.
+            @class.stubs(:retrieve).returns []
+            @class.default_target = "/foo/bar"
+            @class.initvars
+            @class.prefetch
+
+            @filetype = mock 'filetype'
+            Puppet::Util::FileType.filetype(:flat).expects(:new).with("/my/file").returns @filetype
+
+            @filetype.stubs(:write)
+        end
+
+        it "should back up the file being written" do
+            @filetype.expects(:backup)
+
+            @class.flush_target("/my/file")
+        end
+
+        it "should not back up the file more than once between calls to 'prefetch'" do
+            @filetype.expects(:backup).once
+
+            @class.flush_target("/my/file")
+            @class.flush_target("/my/file")
+        end
+
+        it "should back the file up again once the file has been reread" do
+            @filetype.expects(:backup).times(2)
+
+            @class.flush_target("/my/file")
+            @class.prefetch
+            @class.flush_target("/my/file")
+        end
+    end
 end
diff --git a/spec/unit/util/filetype.rb b/spec/unit/util/filetype.rb
index 74dae3356..0506b6b47 100644
--- a/spec/unit/util/filetype.rb
+++ b/spec/unit/util/filetype.rb
@@ -91,12 +91,6 @@ describe Puppet::Util::FileType do
                 Tempfile.stubs(:new).returns @tempfile
             end
 
-            it "should back up the file" do
-                @file.expects(:backup)
-
-                @file.write("foo")
-            end
-
             it "should first create a temp file and copy its contents over to the file location" do
                 Tempfile.expects(:new).with("puppet").returns @tempfile
                 @tempfile.expects(:print).with("my text")