Permalink
Browse files

Merge branch 'pull-1502'

  • Loading branch information...
2 parents 9801de5 + 0216916 commit 1e47c127d62cc2660c27495cdd61dcc85e4884db @adrienthebo adrienthebo committed Mar 6, 2013
Showing with 100 additions and 24 deletions.
  1. +78 −21 lib/puppet/type/file.rb
  2. +22 −3 spec/unit/type/file_spec.rb
View
@@ -597,34 +597,35 @@ def perform_recursion(path)
)
end
- # Remove any existing data. This is only used when dealing with
- # links or directories.
+ # Back up and remove the file or directory at `self[:path]`.
+ #
+ # @param [Symbol] should The file type replacing the current content.
+ # @return [Boolean] True if the file was removed, else False
+ # @raises [fail???] If the current file isn't one of %w{file link directory} and can't be removed.
def remove_existing(should)
- return unless s = stat
+ wanted_type = should.to_s
+ current_type = read_current_type
+
+ if current_type.nil?
+ return false
+ end
- self.fail "Could not back up; will not replace" unless perform_backup
+ if can_backup?(current_type)
+ backup_existing
+ end
- unless should.to_s == "link"
- return if s.ftype.to_s == should.to_s
+ if wanted_type != "link" and current_type == wanted_type
+ return false
end
- case s.ftype
+ case current_type
when "directory"
- if self[:force] == :true
- debug "Removing existing directory for replacement with #{should}"
- FileUtils.rmtree(self[:path])
- else
- notice "Not removing directory; use 'force' to override"
- return
- end
+ return remove_directory(wanted_type)
when "link", "file"
- debug "Removing existing #{s.ftype} for replacement with #{should}"
- ::File.unlink(self[:path])
+ return remove_file(current_type, wanted_type)
else
- self.fail "Could not back up files of type #{s.ftype}"
+ self.fail "Could not back up files of type #{current_type}"
end
- @stat = :needs_stat
- true
end
def retrieve
@@ -746,6 +747,64 @@ def write(property)
private
+ # @return [String] The type of the current file, cast to a string.
+ def read_current_type
+ stat_info = stat
+ if stat_info
+ stat_info.ftype.to_s
+ else
+ nil
+ end
+ end
+
+ # @return [Boolean] If the current file can be backed up and needs to be backed up.
+ def can_backup?(type)
+ if type == "directory" and self[:force] == :false
+ # (#18110) Directories cannot be removed without :force, so it doesn't
+ # make sense to back them up.
+ false
+ else
+ true
+ end
+ end
+
+ # @return [Boolean] True if the directory was removed
+ # @api private
+ def remove_directory(wanted_type)
+ if self[:force] == :true
+ debug "Removing existing directory for replacement with #{wanted_type}"
+ FileUtils.rmtree(self[:path])
+ stat_needed
+ true
+ else
+ notice "Not removing directory; use 'force' to override"
+ false
+ end
+ end
+
+ # @return [Boolean] if the file was removed (which is always true currently)
+ # @api private
+ def remove_file(current_type, wanted_type)
+ debug "Removing existing #{current_type} for replacement with #{wanted_type}"
+ ::File.unlink(self[:path])
+ stat_needed
+ true
+ end
+
+ def stat_needed
+ @stat = :needs_stat
+ end
+
+ # Back up the existing file at a given prior to it being removed
+ # @api private
+ # @raise [Puppet::Error] if the file backup failed
+ # @return [void]
+ def backup_existing
+ unless perform_backup
+ raise Puppet::Error, "Could not back up; will not replace"
+ end
+ end
+
# Should we validate the checksum of the file we're writing?
def validate_checksum?
self[:checksum] !~ /time/
@@ -766,8 +825,6 @@ def write_content(file)
(content = property(:content)) && content.write(file)
end
- private
-
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
@@ -856,20 +856,39 @@
describe "#remove_existing" do
it "should do nothing if the file doesn't exist" do
- file.remove_existing(:file).should == nil
+ file.remove_existing(:file).should == false
end
it "should fail if it can't backup the file" do
- file.stubs(:stat).returns stub('stat')
+ file.stubs(:stat).returns stub('stat', :ftype => 'file')
file.stubs(:perform_backup).returns false
expect { file.remove_existing(:file) }.to raise_error(Puppet::Error, /Could not back up; will not replace/)
end
+ describe "backing up directories" do
+ it "should not backup directories if force is false" do
+ file[:force] = false
+ file.stubs(:stat).returns stub('stat', :ftype => 'directory')
+ file.expects(:perform_backup).never
+ file.remove_existing(:file).should == false
+ end
+
+ it "should backup directories if force is true" do
+ file[:force] = true
+ FileUtils.expects(:rmtree).with(file[:path])
+
+ file.stubs(:stat).returns stub('stat', :ftype => 'directory')
+ file.expects(:perform_backup).once.returns(true)
+
+ file.remove_existing(:file).should == true
+ end
+ end
+
it "should not do anything if the file is already the right type and not a link" do
file.stubs(:stat).returns stub('stat', :ftype => 'file')
- file.remove_existing(:file).should == nil
+ file.remove_existing(:file).should == false
end
it "should not remove directories and should not invalidate the stat unless force is set" do

0 comments on commit 1e47c12

Please sign in to comment.