Adding my first integration test, verifying that

checksum interaction behaves as I expect when
interacting with the file terminus.

I've also changed how files and checksums behave a bit.
Files now create model instances with the content as
the only argument during initialization, and checksums
now calculate their checksums rather than having them passed
in.

8 files changed, 171 insertions, 85 deletions

diff --git a/lib/puppet/checksum.rb b/lib/puppet/checksum.rb
index 65fb7ef76..c607953c1 100644
--- a/lib/puppet/checksum.rb
+++ b/lib/puppet/checksum.rb
@@ -12,26 +12,53 @@ class Puppet::Checksum
 
     indirects :checksum
 
-    attr_accessor :name, :content
-    attr_reader :algorithm
+    attr_reader :algorithm, :content
 
     def algorithm=(value)
-        value = value.intern if value.respond_to?(:intern)
+        unless respond_to?(value)
+            raise ArgumentError, "Checksum algorithm %s is not supported" % value
+        end
+        value = value.intern if value.is_a?(String)
         @algorithm = value
+        # Reset the checksum so it's forced to be recalculated.
+        @checksum = nil
     end
 
-    def initialize(name)
-        raise ArgumentError.new("You must specify the checksum") unless name
-
-        if name =~ /^\{(\w+)\}(.+$)$/
-            @algorithm, @name = $1.intern, $2
-        else
-            @name = name
-            @algorithm = :md5
+    # Calculate (if necessary) and return the checksum
+    def checksum
+        unless @checksum
+            @checksum = send(algorithm)
         end
+        @checksum
+    end
+
+    def initialize(content, algorithm = nil)
+        raise ArgumentError.new("You must specify the content") unless content
+
+        @content = content
+        self.algorithm = algorithm || "md5"
+
+        # Init to avoid warnings.
+        @checksum = nil
+    end
+
+    # This can't be private, else respond_to? returns false.
+    def md5
+        require 'digest/md5'
+        Digest::MD5.hexdigest(content)
+    end
+
+    # This is here so the Indirector::File terminus works correctly.
+    def name
+        checksum
+    end
+
+    def sha1
+        require 'digest/sha1'
+        Digest::SHA1.hexdigest(content)
     end
 
     def to_s
-        "Checksum<{%s}%s>" % [algorithm, name]
+        "Checksum<{%s}%s>" % [algorithm, checksum]
     end
 end
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index dbbe5e29d..f76ae9b84 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -498,7 +498,9 @@ module Puppet
     # we get an infinite loop otherwise.
     self.setdefaults(:main,
         :facts_terminus => ["yaml",
-            "The backend store to use for client facts."]
+            "The backend store to use for client facts."],
+        :checksum_terminus => ["file",
+            "The backend store to use for storing files by checksum (i.e., filebuckets)."]
     )
 
     self.setdefaults(:yaml,
diff --git a/lib/puppet/indirector/file.rb b/lib/puppet/indirector/file.rb
index 4f231e9ec..c2d36c46b 100644
--- a/lib/puppet/indirector/file.rb
+++ b/lib/puppet/indirector/file.rb
@@ -32,9 +32,7 @@ class Puppet::Indirector::File < Puppet::Indirector::Terminus
             raise Puppet::Error, "Could not retrieve path %s: %s" % [path, detail]
         end
 
-        file = model.new(name)
-        file.content = content
-        return file
+        return model.new(content)
     end
 
     def save(file)
diff --git a/spec/integration/checksum.rb b/spec/integration/checksum.rb
new file mode 100755
index 000000000..f112f7502
--- /dev/null
+++ b/spec/integration/checksum.rb
@@ -0,0 +1,48 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2007-9-22.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../spec_helper'
+
+require 'puppet/checksum'
+
+describe Puppet::Checksum, " when using the file terminus" do
+    before do
+        Puppet[:checksum_terminus] = "file"
+
+        @content = "this is some content"
+        @sum = Puppet::Checksum.new(@content)
+
+        @file = Puppet::Checksum.indirection.terminus.path(@sum.checksum)
+    end
+
+    it "should store content at a path determined by its checksum" do
+        File.stubs(:directory?).returns(true)
+        filehandle = mock 'filehandle'
+        filehandle.expects(:print).with(@content)
+        File.expects(:open).with(@file, "w").yields(filehandle)
+
+        @sum.save
+    end
+
+    it "should retrieve stored content when the checksum is provided as the key" do
+        File.stubs(:exist?).returns(true)
+        File.expects(:read).with(@file).returns(@content)
+
+        newsum = Puppet::Checksum.find(@sum.checksum)
+
+        newsum.content.should == @content
+    end
+
+    it "should remove specified files when asked" do
+        File.stubs(:exist?).returns(true)
+        File.expects(:unlink).with(@file)
+
+        Puppet::Checksum.destroy(@sum)
+    end
+
+    after do
+        Puppet.settings.clear
+    end
+end
diff --git a/spec/unit/indirector/file.rb b/spec/unit/indirector/file.rb
index 176832880..cc86f9fa9 100755
--- a/spec/unit/indirector/file.rb
+++ b/spec/unit/indirector/file.rb
@@ -27,26 +27,25 @@ describe Puppet::Indirector::File, " when finding files" do
     include FileTerminusTesting
 
     it "should provide a method to return file contents at a specified path" do
+        @searcher.should respond_to(:find)
     end
 
     it "should return file contents as an instance of the model" do
         content = "my content"
 
         file = mock 'file'
-        @model.expects(:new).with(@path).returns(file)
-        file.expects(:content=).with(content)
+        @model.expects(:new).with(content).returns(file)
 
         File.expects(:exist?).with(@path).returns(true)
         File.expects(:read).with(@path).returns(content)
         @searcher.find(@path)
     end
 
-    it "should set the file contents as the 'content' attribute of the returned instance" do
+    it "should create the model instance with the content as the only argument to initialization" do
         content = "my content"
 
         file = mock 'file'
-        @model.expects(:new).with(@path).returns(file)
-        file.expects(:content=).with(content)
+        @model.expects(:new).with(content).returns(file)
 
         File.expects(:exist?).with(@path).returns(true)
         File.expects(:read).with(@path).returns(content)
@@ -72,23 +71,6 @@ describe Puppet::Indirector::File, " when finding files" do
         File.expects(:exist?).with(@path.upcase).returns(false)
         @searcher.find(@path)
     end
-
-    it "should use the passed-in name for the model instance even if a path() method exists" do
-        @searcher.meta_def(:path) do |name|
-            name.upcase
-        end
-
-        content = "something"
-        file = mock 'file'
-        file.expects(:content=).with(content)
-
-        # The passed-in path, rather than the upcased version
-        @model.expects(:new).with(@path).returns(file)
-
-        File.expects(:exist?).with(@path.upcase).returns(true)
-        File.expects(:read).with(@path.upcase).returns(content)
-        @searcher.find(@path).should equal(file)
-    end
 end
 
 describe Puppet::Indirector::File, " when saving files" do
diff --git a/spec/unit/indirector/file/checksum.rb b/spec/unit/indirector/file/checksum.rb
index 950a377a7..3a4cd3d61 100755
--- a/spec/unit/indirector/file/checksum.rb
+++ b/spec/unit/indirector/file/checksum.rb
@@ -7,6 +7,22 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/indirector/file/checksum'
 
+module FileChecksumTesting
+    def setup
+        Puppet.settings.stubs(:use)
+        @store = Puppet::Indirector::File::Checksum.new
+
+        @value = "70924d6fa4b2d745185fa4660703a5c0"
+        @sum = stub 'sum', :name => @value
+
+        @dir = "/what/ever"
+
+        Puppet.stubs(:[]).with(:bucketdir).returns(@dir)
+
+        @path = @store.path(@value)
+    end
+end
+
 describe Puppet::Indirector::File::Checksum do
     it "should be a subclass of the File terminus class" do
         Puppet::Indirector::File::Checksum.superclass.should equal(Puppet::Indirector::File)
@@ -25,18 +41,7 @@ describe Puppet::Indirector::File::Checksum, " when initializing" do
 end
 
 describe Puppet::Indirector::File::Checksum, " when determining file paths" do
-    before do
-        Puppet.settings.stubs(:use)
-        @store = Puppet::Indirector::File::Checksum.new
-
-        @value = "70924d6fa4b2d745185fa4660703a5c0"
-
-        @dir = "/what/ever"
-
-        Puppet.stubs(:[]).with(:bucketdir).returns(@dir)
-
-        @path = @store.path(@value)
-    end
+    include FileChecksumTesting
 
     # I was previously passing the object in.
     it "should use the value passed in to path() as the checksum" do
@@ -66,22 +71,6 @@ describe Puppet::Indirector::File::Checksum, " when determining file paths" do
     end
 end
 
-module FileChecksumTesting
-    def setup
-        Puppet.settings.stubs(:use)
-        @store = Puppet::Indirector::File::Checksum.new
-
-        @value = "70924d6fa4b2d745185fa4660703a5c0"
-        @sum = stub 'sum', :name => @value
-
-        @dir = "/what/ever"
-
-        Puppet.stubs(:[]).with(:bucketdir).returns(@dir)
-
-        @path = @store.path(@value)
-    end
-end
-
 describe Puppet::Indirector::File::Checksum, " when retrieving files" do
     include FileChecksumTesting
 
@@ -91,7 +80,7 @@ describe Puppet::Indirector::File::Checksum, " when retrieving files" do
         @store.find(@value)
     end
 
-    it "should return an instance of Puppet::Checksum with the name and content set correctly if the file exists" do
+    it "should return an instance of Puppet::Checksum with the checksum and content set correctly if the file exists" do
         content = "my content"
         sum = stub 'file'
         sum.expects(:content=).with(content)
diff --git a/spec/unit/indirector/indirector.rb b/spec/unit/indirector/indirector.rb
index 47e93cbaa..1702bf51f 100755
--- a/spec/unit/indirector/indirector.rb
+++ b/spec/unit/indirector/indirector.rb
@@ -1,4 +1,7 @@
+#!/usr/bin/env ruby
+
 require File.dirname(__FILE__) + '/../../spec_helper'
+
 require 'puppet/defaults'
 require 'puppet/indirector'
 
@@ -36,12 +39,17 @@ describe Puppet::Indirector, "when registering an indirection" do
         Proc.new { @thingie.indirects :second }.should raise_error(ArgumentError)
     end
 
+    it "should make the indirection available via an accessor" do
+        @indirection = @thingie.indirects :first
+        @thingie.indirection.should equal(@indirection)
+    end
+
     after do
         @indirection.delete if @indirection
     end
 end
 
-describe Puppet::Indirector, " when redirecting model" do
+describe Puppet::Indirector, " when redirecting a model" do
   before do
     @thingie = Class.new do
       extend Puppet::Indirector
@@ -51,22 +59,22 @@ describe Puppet::Indirector, " when redirecting model" do
     @thingie.expects(:indirection).returns(@mock_terminus)
   end
   
-  it "should give model the ability to lookup a model instance by letting the indirection perform the lookup" do
+  it "should give the model the ability to lookup a model instance by letting the indirection perform the lookup" do
     @mock_terminus.expects(:find)
     @thingie.find
   end
 
-  it "should give model the ability to remove model instances from a terminus by letting the indirection remove the instance" do
+  it "should give the model the ability to remove model instances from a terminus by letting the indirection remove the instance" do
     @mock_terminus.expects(:destroy)
     @thingie.destroy  
   end
   
-  it "should give model the ability to search for model instances by letting the indirection find the matching instances" do
+  it "should give the model the ability to search for model instances by letting the indirection find the matching instances" do
     @mock_terminus.expects(:search)
     @thingie.search    
   end
   
-  it "should give model the ability to store a model instance by letting the indirection store the instance" do
+  it "should give the model the ability to store a model instance by letting the indirection store the instance" do
     thing = @thingie.new
     @mock_terminus.expects(:save).with(thing)
     thing.save        
diff --git a/spec/unit/other/checksum.rb b/spec/unit/other/checksum.rb
index 5610870a6..6a63e833d 100755
--- a/spec/unit/other/checksum.rb
+++ b/spec/unit/other/checksum.rb
@@ -9,35 +9,67 @@ require 'puppet/checksum'
 
 describe Puppet::Checksum do
     it "should have 'Checksum' and the checksum algorithm when converted to a string" do
-        sum = Puppet::Checksum.new("whatever")
-        sum.algorithm = "yay"
-        sum.to_s.should == "Checksum<{yay}whatever>"
+        inst = Puppet::Checksum.new("whatever", "md5")
+        inst.to_s.should == "Checksum<{md5}#{inst.checksum}>"
     end
 
     it "should convert algorithm names to symbols when they are set after checksum creation" do
         sum = Puppet::Checksum.new("whatever")
-        sum.algorithm = "yay"
-        sum.algorithm.should == :yay
+        sum.algorithm = "md5"
+        sum.algorithm.should == :md5
+    end
+
+    it "should return the checksum as the name" do
+        sum = Puppet::Checksum.new("whatever")
+        sum.checksum.should == sum.name
     end
 end
 
 describe Puppet::Checksum, " when initializing" do
-    it "should require a name" do
+    before do
+        @content = "this is some content"
+        @sum = Puppet::Checksum.new(@content)
+    end
+
+    it "should require content" do
         proc { Puppet::Checksum.new(nil) }.should raise_error(ArgumentError)
     end
 
-    it "should set the name appropriately" do
-        Puppet::Checksum.new("whatever").name.should == "whatever"
+    it "should set the content appropriately" do
+        @sum.content.should == @content
+    end
+
+    it "should calculate the checksum" do
+        require 'digest/md5'
+        Digest::MD5.expects(:hexdigest).with(@content).returns(:mychecksum)
+        @sum.checksum.should == :mychecksum
+    end
+
+    it "should not calculate the checksum until it is asked for" do
+        require 'digest/md5'
+        Digest::MD5.expects(:hexdigest).never
+        sum = Puppet::Checksum.new(@content, :md5)
     end
 
-    it "should parse checksum algorithms out of the name if they are there" do
-        sum = Puppet::Checksum.new("{other}whatever")
-        sum.algorithm.should == :other
-        sum.name.should == "whatever"
+    it "should remove the old checksum value if the algorithm is changed" do
+        Digest::MD5.expects(:hexdigest).with(@content).returns(:oldsum)
+        oldsum = @sum.checksum
+        @sum.algorithm = :sha1
+        Digest::SHA1.expects(:hexdigest).with(@content).returns(:newsum)
+        @sum.checksum.should == :newsum
     end
 
     it "should default to 'md5' as the checksum algorithm if the algorithm is not in the name" do
-        Puppet::Checksum.new("whatever").algorithm.should == :md5
+        @sum.algorithm.should == :md5
+    end
+
+    it "should support specifying the algorithm during initialization" do
+        sum = Puppet::Checksum.new(@content, :sha1)
+        sum.algorithm.should == :sha1
+    end
+
+    it "should fail when an unsupported algorithm is used" do
+        proc { Puppet::Checksum.new(@content, :nope) }.should raise_error(ArgumentError)
     end
 end