For #3822 - Reducing checksum code duplication

The FileBucket code had a bunch of checksum code
that was already available in a library, and it used a
checksum format (type + data) that was incompatible with
what we were using everywhere else.

This just fixes that code duplication.

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

1 files changed, 56 insertions, 43 deletions

diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb
index 1e95e5ef6..52ee4f711 100644
--- a/lib/puppet/file_bucket/file.rb
+++ b/lib/puppet/file_bucket/file.rb
@@ -1,7 +1,10 @@
 require 'puppet/file_bucket'
 require 'puppet/indirector'
+require 'puppet/util/checksums'
 
 class Puppet::FileBucket::File
+    include Puppet::Util::Checksums
+
     # This class handles the abstract notion of a file in a filebucket.
     # There are mechanisms to save and load this file locally and remotely in puppet/indirector/filebucketfile/*
     # There is a compatibility class that emulates pre-indirector filebuckets in Puppet::FileBucket::Dipper
@@ -16,16 +19,18 @@ class Puppet::FileBucket::File
     attr :bucket_path, true
 
     def self.default_checksum_type
-        :md5
+        "md5"
     end
 
     def initialize( contents, options = {} )
-        @contents      = contents
         @bucket_path   = options[:bucket_path]
         @path          = options[:path]
         @paths         = options[:paths] || []
+
         @checksum      = options[:checksum]
-        @checksum_type = options[:checksum_type] || self.class.default_checksum_type
+        @checksum_type = options[:checksum_type]
+
+        self.contents  = contents
 
         yield(self) if block_given?
 
@@ -33,61 +38,45 @@ class Puppet::FileBucket::File
     end
 
     def validate!
-        digest_class( @checksum_type ) # raises error on bad types
-        raise ArgumentError, 'contents must be a string' unless @contents.is_a?(String)
-        validate_checksum(@checksum) if @checksum
+        validate_checksum_type!(checksum_type)
+        validate_checksum!(checksum) if checksum
     end
 
-    def contents=(contents)
+    def contents=(str)
         raise "You may not change the contents of a FileBucket File" if @contents
-        @contents = contents
-    end
-
-    def checksum=(checksum)
-        validate_checksum(checksum)
-        self.checksum_type = checksum # this grabs the prefix only
-        @checksum = checksum
-    end
-
-    def validate_checksum(new_checksum)
-        unless new_checksum == checksum_of_type(new_checksum)
-            raise Puppet::Error, "checksum does not match contents"
-        end
+        validate_content!(str)
+        @contents = str
     end
 
     def checksum
-        @checksum ||= checksum_of_type(checksum_type)
+        return @checksum if @checksum
+        @checksum = calculate_checksum if contents
+        @checksum
     end
 
-    def checksum_of_type( type )
-        type = checksum_type( type ) # strip out data segment if there is one
-        type.to_s + ":" + digest_class(type).hexdigest(@contents)
+    def checksum=(checksum)
+        validate_checksum!(checksum)
+        @checksum = checksum
     end
 
     def checksum_type=( new_checksum_type )
         @checksum = nil
-        @checksum_type = checksum_type(new_checksum_type)
-    end
-
-    def checksum_type(checksum = @checksum_type)
-        checksum.to_s.split(':',2)[0].to_sym
+        @checksum_type = new_checksum_type
     end
 
-    def checksum_data(new_checksum = self.checksum)
-        new_checksum.split(':',2)[1]
-    end
-
-    def checksum_data=(new_data)
-        self.checksum = "#{checksum_type}:#{new_data}"
+    def checksum_type
+        unless @checksum_type
+            if @checksum
+                @checksum_type = sumtype(checksum)
+            else
+                @checksum_type = self.class.default_checksum_type
+            end
+        end
+        @checksum_type
     end
 
-    def digest_class(type = nil)
-        case checksum_type(type)
-        when :md5  : require 'digest/md5'  ; Digest::MD5
-        when :sha1 : require 'digest/sha1' ; Digest::SHA1
-        else
-            raise ArgumentError, "not a known checksum type: #{checksum_type(type)}"
-        end
+    def checksum_data
+        sumdata(checksum)
     end
 
     def to_s
@@ -99,7 +88,10 @@ class Puppet::FileBucket::File
     end
 
     def name=(name)
-        self.checksum_type, self.checksum_data, self.path = name.split('/',3)
+        data = name.split('/',3)
+        self.path = data.pop
+        @checksum_type = nil
+        self.checksum = "{#{data[0]}}#{data[1]}"
     end
 
     def conflict_check?
@@ -120,4 +112,25 @@ class Puppet::FileBucket::File
         self.new( pson["contents"], :path => pson["path"] )
     end
 
+    private
+
+    def calculate_checksum
+        "{#{checksum_type}}" + send(checksum_type, contents)
+    end
+
+    def validate_content!(content)
+        raise ArgumentError, "Contents must be a string" if content and ! content.is_a?(String)
+    end
+
+    def validate_checksum!(new_checksum)
+        newtype = sumtype(new_checksum)
+
+        unless sumdata(new_checksum) == (calc_sum = send(newtype, contents))
+            raise Puppet::Error, "Checksum #{new_checksum} does not match contents #{calc_sum}"
+        end
+    end
+
+    def validate_checksum_type!(type)
+        raise ArgumentError, "Invalid checksum type #{type}" unless respond_to?(type)
+    end
 end