Fixing #505, #508, and #513.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2219 980ebf18-57e1-0310-9a29-db15c13687c0

3 files changed, 117 insertions, 17 deletions

diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index ad21c5c55..116c8880d 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -36,10 +36,12 @@ module Puppet
                 a ``filebucket``, which stores files by their MD5 sums and allows
                 easy retrieval without littering directories with backups.  You
                 can specify a local filebucket or a network-accessible
-                server-based filebucket.  Alternatively, if you specify any
-                value that begins with a ``.`` (e.g., ``.puppet-bak``), then
-                Puppet will use copy the file in the same directory with that
-                value as the extension of the backup.
+                server-based filebucket by setting ``backup => bucket-name``.
+                Alternatively, if you specify any value that begins with a ``.``
+                (e.g., ``.puppet-bak``), then Puppet will use copy the file in
+                the same directory with that value as the extension of the
+                backup. Setting ``backup => false`` disables all backups of the
+                file in question.
                 
                 Puppet automatically creates a local filebucket named ``puppet`` and
                 defaults to backing up there.  To use a server-based filebucket,
@@ -114,13 +116,19 @@ module Puppet
                 management."
 
             newvalues(:true, :false, :inf, /^[0-9]+$/)
+
+            # Replace the validation so that we allow numbers in
+            # addition to string representations of them.
+            validate { |arg| }
             munge do |value|
                 newval = super(value)
                 case newval
                 when :true, :inf: true
                 when :false: false
+                when Integer, Fixnum, Bignum: value
+                when /^\d+$/: Integer(value)
                 else
-                    newval
+                    raise ArgumentError, "Invalid recurse value %s" % value.inspect
                 end
             end
         end
@@ -1045,6 +1053,9 @@ module Puppet
                 end
             end
 
+            # make sure all of the modes are actually correct
+            property_fix
+
             # And then update our checksum, so the next run doesn't find it.
             # FIXME This is extra work, because it's going to read the whole
             # file back in again.
@@ -1069,6 +1080,23 @@ module Puppet
             #     yield
             # end
         end
+
+        private
+        # There are some cases where all of the work does not get done on
+        # file creation/modification, so we have to do some extra checking.
+        def property_fix
+            self.each do |thing|
+                next unless thing.is_a? Puppet::Property
+                next unless [:mode, :owner, :group].include?(thing.name)
+
+                # Make sure we get a new stat objct
+                self.stat(true)
+                thing.retrieve
+                unless thing.insync?
+                    thing.sync
+                end
+            end
+        end
     end # Puppet.type(:pfile)
 
     # the filesource class can't include the path, because the path
diff --git a/lib/puppet/type/pfile/ensure.rb b/lib/puppet/type/pfile/ensure.rb
index 908c07df1..076dfbdcf 100755
--- a/lib/puppet/type/pfile/ensure.rb
+++ b/lib/puppet/type/pfile/ensure.rb
@@ -76,6 +76,7 @@ module Puppet
                     Dir.mkdir(@parent[:path])
                 end
             end
+            @parent.send(:property_fix)
             @parent.setchecksum
             return :directory_created
         end
@@ -156,18 +157,6 @@ module Puppet
 
             event = super
 
-            # There are some cases where all of the work does not get done on
-            # file creation, so we have to do some extra checking.
-            @parent.each do |thing|
-                next unless thing.is_a? Puppet::Property
-                next if thing == self
-
-                thing.retrieve
-                unless thing.insync?
-                    thing.sync
-                end
-            end
-
             return event
         end
     end
diff --git a/test/ral/types/file.rb b/test/ral/types/file.rb
index f616aac4c..b7f6cda2e 100755
--- a/test/ral/types/file.rb
+++ b/test/ral/types/file.rb
@@ -1978,6 +1978,89 @@ class TestFile < Test::Unit::TestCase
             end
         end
     end
+
+    # Testing #508
+    if Process.uid == 0
+    def test_files_replace_with_right_attrs
+        source = tempfile()
+        File.open(source, "w") { |f|
+            f.puts "some text"
+        }
+        File.chmod(0755, source)
+        user = nonrootuser
+        group = nonrootgroup
+        path = tempfile()
+        good = {:uid => user.uid, :gid => group.gid, :mode => 0640}
+
+        run = Proc.new do |obj, msg|
+            assert_apply(obj)
+            stat = File.stat(obj[:path])
+            good.each do |should, sval|
+                if should == :mode
+                    current = filemode(obj[:path])
+                else
+                    current = stat.send(should)
+                end
+                assert_equal(sval, current,
+                    "Attr %s was not correct %s" % [should, msg])
+            end
+        end
+
+        file = Puppet::Type.newfile(:path => path, :owner => user.name,
+            :group => group.name, :mode => 0640, :backup => false)
+        {:source => source, :content => "some content"}.each do |attr, value|
+            file[attr] = value
+            # First create the file
+            run.call(file, "upon creation with %s" % attr)
+
+            # Now change something so that we replace the file
+            case attr
+            when :source:
+                    File.open(source, "w") { |f| f.puts "some different text" }
+            when :content: file[:content] = "something completely different"
+            else
+                raise "invalid attr %s" % attr
+            end
+            
+            # Run it again
+            run.call(file, "after modification with %s" % attr)
+
+            # Now remove the file and the attr
+            file.delete(attr)
+            File.unlink(path)
+        end
+    end
+    end
+
+    # #505
+    def test_numeric_recurse
+        dir = tempfile()
+        subdir = File.join(dir, "subdir")
+        other = File.join(subdir, "deeper")
+        file = File.join(other, "file")
+        [dir, subdir, other].each { |d| Dir.mkdir(d) }
+        File.open(file, "w") { |f| f.puts "yay" }
+        File.chmod(0644, file)
+        obj = Puppet::Type.newfile(:path => dir, :mode => 0750, :recurse => "2")
+
+        children = nil
+        assert_nothing_raised("Failure when recursing") do
+            children = obj.eval_generate
+        end
+        assert(obj.class[subdir], "did not create subdir object")
+        children.each do |c|
+            assert_nothing_raised("Failure when recursing on %s" % c) do
+                others = c.eval_generate
+            end
+        end
+        oobj = obj.class[other]
+        assert(oobj, "did not create other object")
+
+        assert_nothing_raised do
+            assert_nil(oobj.eval_generate, "recursed too far")
+        end
+
+    end
 end
 
 # $Id$