[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35
test branch
puppet-dev at googlegroups.com
Wed Jul 14 10:34:04 UTC 2010
The following commit has been merged in the upstream branch:
commit 3b7aac584f8c0fe34079a1b34d301c62ce068ece
Author: Luke Kanies <luke at puppetlabs.com>
Date: Thu May 20 17:06:16 2010 -0700
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 at puppetlabs.com>
diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb
index 1e95e5e..52ee4f7 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
diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb
index 4c08b8d..9fe40f6 100644
--- a/lib/puppet/indirector/file_bucket_file/file.rb
+++ b/lib/puppet/indirector/file_bucket_file/file.rb
@@ -1,8 +1,11 @@
require 'puppet/indirector/code'
require 'puppet/file_bucket/file'
+require 'puppet/util/checksums'
module Puppet::FileBucketFile
class File < Puppet::Indirector::Code
+ include Puppet::Util::Checksums
+
desc "Store files in a directory set based on their checksums."
def initialize
@@ -80,9 +83,11 @@ module Puppet::FileBucketFile
end
def request_to_checksum_and_path( request )
- checksum_type, checksum, path = request.key.split(/[:\/]/, 3)
+ return [request.key, nil] if checksum?(request.key)
+
+ checksum_type, checksum, path = request.key.split(/\//, 3)
return nil if checksum_type.to_s == ""
- return [ checksum_type + ":" + checksum, path ]
+ return [ "{#{checksum_type}}#{checksum}", path ]
end
def path_for(bucket_file, subfile = nil)
diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
index a095237..41f8786 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -7,6 +7,15 @@ module Puppet::Util::Checksums
end
# Strip the checksum type from an existing checksum
+ def sumdata(checksum)
+ if checksum =~ /^\{(\w+)\}(.+)/
+ return $2
+ else
+ return nil
+ end
+ end
+
+ # Strip the checksum type from an existing checksum
def sumtype(checksum)
if checksum =~ /^\{(\w+)\}/
return $1
diff --git a/spec/unit/file_bucket/file.rb b/spec/unit/file_bucket/file.rb
index 76f8e25..7358765 100644
--- a/spec/unit/file_bucket/file.rb
+++ b/spec/unit/file_bucket/file.rb
@@ -12,66 +12,18 @@ describe Puppet::FileBucket::File do
Puppet[:bucketdir] = "/dev/null/bucket"
@digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
- @checksum = "md5:4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
+ @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
@dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
@contents = "file contents"
end
- it "should save a file" do
- ::File.expects(:exist?).with("#{@dir}/contents").returns false
- ::File.expects(:directory?).with(@dir).returns false
- ::FileUtils.expects(:mkdir_p).with(@dir)
- ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
-
- bucketfile = Puppet::FileBucket::File.new(@contents)
- bucketfile.save
-
- end
-
- describe "using the indirector's find method" do
- it "should return nil if a file doesn't exist" do
- ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
- bucketfile = Puppet::FileBucket::File.find("md5:#{@digest}")
- bucketfile.should == nil
- end
-
- it "should find a filebucket if the file exists" do
- ::File.expects(:exist?).with("#{@dir}/contents").returns true
- ::File.expects(:exist?).with("#{@dir}/paths").returns false
- ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
- bucketfile = Puppet::FileBucket::File.find("md5:#{@digest}")
- bucketfile.should_not == nil
- end
-
- describe "using RESTish digest notation" do
- it "should return nil if a file doesn't exist" do
- ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
- bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
- bucketfile.should == nil
- end
-
- it "should find a filebucket if the file exists" do
- ::File.expects(:exist?).with("#{@dir}/contents").returns true
- ::File.expects(:exist?).with("#{@dir}/paths").returns false
- ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
- bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
- bucketfile.should_not == nil
- end
-
- end
- end
-
it "should have a to_s method to return the contents" do
Puppet::FileBucket::File.new(@contents).to_s.should == @contents
end
- it "should have a method that returns the digest algorithm" do
- Puppet::FileBucket::File.new(@contents, :checksum => @checksum).checksum_type.should == :md5
+ it "should calculate the checksum type from the passed in checksum" do
+ Puppet::FileBucket::File.new(@contents, :checksum => @checksum).checksum_type.should == "md5"
end
it "should allow contents to be specified in a block" do
@@ -102,23 +54,21 @@ describe Puppet::FileBucket::File do
Puppet::FileBucket::File.new(@contents).contents.should == @contents
end
+ it "should default to 'md5' as the checksum algorithm if the algorithm is not in the name" do
+ Puppet::FileBucket::File.new(@contents).checksum_type.should == "md5"
+ end
+
it "should calculate the checksum" do
- Digest::MD5.expects(:hexdigest).with(@contents).returns('mychecksum')
- Puppet::FileBucket::File.new(@contents).checksum.should == 'md5:mychecksum'
+ Puppet::FileBucket::File.new(@contents).checksum.should == @checksum
end
it "should remove the old checksum value if the algorithm is changed" do
- Digest::MD5.expects(:hexdigest).with(@contents).returns('oldsum')
sum = Puppet::FileBucket::File.new(@contents)
- oldsum = sum.checksum
+ sum.checksum.should_not be_nil
+ newsum = Digest::SHA1.hexdigest(@contents).to_s
sum.checksum_type = :sha1
- Digest::SHA1.expects(:hexdigest).with(@contents).returns('newsum')
- sum.checksum.should == 'sha1:newsum'
- end
-
- it "should default to 'md5' as the checksum algorithm if the algorithm is not in the name" do
- Puppet::FileBucket::File.new(@contents).checksum_type.should == :md5
+ sum.checksum.should == "{sha1}#{newsum}"
end
it "should support specifying the checksum_type during initialization" do
@@ -130,21 +80,15 @@ describe Puppet::FileBucket::File do
proc { Puppet::FileBucket::File.new(@contents, :checksum_type => :nope) }.should raise_error(ArgumentError)
end
- it "should fail if given an invalid checksum at initialization" do
- proc { Puppet::FileBucket::File.new(@contents, :checksum => "md5:00000000000000000000000000000000") }.should raise_error(RuntimeError)
+ it "should fail if given an checksum at initialization that does not match the contents" do
+ proc { Puppet::FileBucket::File.new(@contents, :checksum => "{md5}00000000000000000000000000000000") }.should raise_error(RuntimeError)
end
- it "should fail if assigned an invalid checksum " do
+ it "should fail if assigned a checksum that does not match the contents" do
bucket = Puppet::FileBucket::File.new(@contents)
- proc { bucket.checksum = "md5:00000000000000000000000000000000" }.should raise_error(RuntimeError)
+ proc { bucket.checksum = "{md5}00000000000000000000000000000000" }.should raise_error(RuntimeError)
end
- it "should accept checksum_data without a prefix" do
- bucket = Puppet::FileBucket::File.new(@contents)
- bucket.checksum_data = @digest
- end
-
-
describe "when using back-ends" do
it "should redirect using Puppet::Indirector" do
Puppet::Indirector::Indirection.instance(:file_bucket_file).model.should equal(Puppet::FileBucket::File)
@@ -185,26 +129,26 @@ describe Puppet::FileBucket::File do
Puppet::FileBucket::File.new(@contents).save
end
- end
- it "should accept a path" do
- remote_path = '/path/on/the/remote/box'
- Puppet::FileBucket::File.new(@contents, :path => remote_path).path.should == remote_path
- end
+ it "should append the path to the paths file" do
+ remote_path = '/path/on/the/remote/box'
- it "should append the path to the paths file" do
- remote_path = '/path/on/the/remote/box'
+ ::File.expects(:directory?).with(@dir).returns(true)
+ ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
+ ::File.expects(:exist?).with("#{@dir}/contents").returns false
- ::File.expects(:directory?).with(@dir).returns(true)
- ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
- ::File.expects(:exist?).with("#{@dir}/contents").returns false
+ mockfile = mock "file"
+ mockfile.expects(:puts).with('/path/on/the/remote/box')
+ ::File.expects(:exist?).with("#{@dir}/paths").returns false
+ ::File.expects(:open).with("#{@dir}/paths", ::File::WRONLY|::File::CREAT|::File::APPEND).yields mockfile
+ Puppet::FileBucket::File.new(@contents, :path => remote_path).save
- mockfile = mock "file"
- mockfile.expects(:puts).with('/path/on/the/remote/box')
- ::File.expects(:exist?).with("#{@dir}/paths").returns false
- ::File.expects(:open).with("#{@dir}/paths", ::File::WRONLY|::File::CREAT|::File::APPEND).yields mockfile
- Puppet::FileBucket::File.new(@contents, :path => remote_path).save
+ end
+ end
+ it "should accept a path" do
+ remote_path = '/path/on/the/remote/box'
+ Puppet::FileBucket::File.new(@contents, :path => remote_path).path.should == remote_path
end
it "should return a url-ish name" do
@@ -219,7 +163,7 @@ describe Puppet::FileBucket::File do
it "should accept a url-ish name" do
bucket = Puppet::FileBucket::File.new(@contents)
lambda { bucket.name = "sha1/034fa2ed8e211e4d20f20e792d777f4a30af1a93/new/path" }.should_not raise_error
- bucket.checksum_type.should == :sha1
+ bucket.checksum_type.should == "sha1"
bucket.checksum_data.should == '034fa2ed8e211e4d20f20e792d777f4a30af1a93'
bucket.path.should == "new/path"
end
@@ -236,4 +180,51 @@ describe Puppet::FileBucket::File do
Puppet::FileBucket::File.from_pson({"contents"=>"file contents"}).contents.should == "file contents"
end
+ it "should save a file" do
+ ::File.expects(:exist?).with("#{@dir}/contents").returns false
+ ::File.expects(:directory?).with(@dir).returns false
+ ::FileUtils.expects(:mkdir_p).with(@dir)
+ ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
+
+ bucketfile = Puppet::FileBucket::File.new(@contents)
+ bucketfile.save
+
+ end
+
+ describe "using the indirector's find method" do
+ it "should return nil if a file doesn't exist" do
+ ::File.expects(:exist?).with("#{@dir}/contents").returns false
+
+ bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+ bucketfile.should == nil
+ end
+
+ it "should find a filebucket if the file exists" do
+ ::File.expects(:exist?).with("#{@dir}/contents").returns true
+ ::File.expects(:exist?).with("#{@dir}/paths").returns false
+ ::File.expects(:read).with("#{@dir}/contents").returns @contents
+
+ bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+ bucketfile.should_not == nil
+ end
+
+ describe "using RESTish digest notation" do
+ it "should return nil if a file doesn't exist" do
+ ::File.expects(:exist?).with("#{@dir}/contents").returns false
+
+ bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
+ bucketfile.should == nil
+ end
+
+ it "should find a filebucket if the file exists" do
+ ::File.expects(:exist?).with("#{@dir}/contents").returns true
+ ::File.expects(:exist?).with("#{@dir}/paths").returns false
+ ::File.expects(:read).with("#{@dir}/contents").returns @contents
+
+ bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
+ bucketfile.should_not == nil
+ end
+
+ end
+ end
end
diff --git a/spec/unit/indirector/file_bucket_file/file.rb b/spec/unit/indirector/file_bucket_file/file.rb
index 6e0d340..bad6de3 100755
--- a/spec/unit/indirector/file_bucket_file/file.rb
+++ b/spec/unit/indirector/file_bucket_file/file.rb
@@ -27,7 +27,7 @@ describe Puppet::FileBucketFile::File do
Puppet[:bucketdir] = "/dev/null/bucket"
@digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
- @checksum = "md5:4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
+ @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
@dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
@contents = "file contents"
@@ -36,7 +36,7 @@ describe Puppet::FileBucketFile::File do
it "should return nil if a file doesn't exist" do
::File.expects(:exist?).with("#{@dir}/contents").returns false
- bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "md5:#{@digest}", {})
+ bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
bucketfile.should == nil
end
@@ -45,7 +45,7 @@ describe Puppet::FileBucketFile::File do
::File.expects(:exist?).with("#{@dir}/paths").returns false
::File.expects(:read).with("#{@dir}/contents").returns @contents
- bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "md5:#{@digest}", {})
+ bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
bucketfile.should_not == nil
end
@@ -59,7 +59,7 @@ describe Puppet::FileBucketFile::File do
mockfile.expects(:readlines).returns( paths )
::File.expects(:open).with("#{@dir}/paths").yields mockfile
- Puppet::FileBucketFile::File.new.send(:find_by_checksum, "md5:#{@digest}", {}).paths.should == paths
+ Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}).paths.should == paths
end
end
@@ -83,7 +83,7 @@ describe Puppet::FileBucketFile::File do
end
it "should call find_by_checksum" do
- @store.expects(:find_by_checksum).with{|x,opts| x == "md5:#{@digest}"}.returns(false)
+ @store.expects(:find_by_checksum).with{|x,opts| x == "{md5}#{@digest}"}.returns(false)
@store.find(@request)
end
@@ -101,7 +101,7 @@ describe Puppet::FileBucketFile::File do
bucketfile.stubs(:checksum).returns(@checksum)
bucketfile.expects(:contents=).with(content)
- Puppet::FileBucket::File.expects(:new).with(nil, {:checksum => "md5:#{@digest}"}).yields(bucketfile).returns(bucketfile)
+ Puppet::FileBucket::File.expects(:new).with(nil, {:checksum => "{md5}#{@digest}"}).yields(bucketfile).returns(bucketfile)
::File.expects(:exist?).with(@contents_path).returns(true)
::File.expects(:exist?).with(@paths_path).returns(false)
@@ -164,7 +164,7 @@ describe Puppet::FileBucketFile::File do
Puppet[:bucketdir] = "/dev/null/bucket"
@digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
- @checksum = "md5:4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
+ @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
@dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
@contents = "file contents"
@@ -206,7 +206,7 @@ describe Puppet::FileBucketFile::File do
Puppet[:bucketdir] = "/dev/null/bucket"
@digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
- @checksum = "md5:4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
+ @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
@dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
@contents = "file contents"
diff --git a/spec/unit/util/checksums.rb b/spec/unit/util/checksums.rb
index eba5643..35d1863 100755
--- a/spec/unit/util/checksums.rb
+++ b/spec/unit/util/checksums.rb
@@ -54,6 +54,10 @@ describe Puppet::Util::Checksums do
@summer.sumtype("{md5}asdfasdfa").should == "md5"
end
+ it "should have a method for stripping the data from a checksum" do
+ @summer.sumdata("{md5}asdfasdfa").should == "asdfasdfa"
+ end
+
it "should return a nil sumtype if the checksum does not mention a checksum type" do
@summer.sumtype("asdfasdfa").should be_nil
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list