Intermediate commit.

This commit adds a Request instance into the indirection,
pushing it all the way to the terminus instances.  It's
a big commit because it requires modifying every terminus class.

There are still some thorny design issues.  In particular, who
should be responsible for making the request object?  I've tried
having both the indirection class and the Indirector module creating
it, and both have their issues.

Also, the Catalog class previously allowed passing Node instances
directly to the find method, which is now no longer possible because
the Request class would treat the node as the instance being found.
We need the request class to have two modes, one when it's passed an
instance and one when it's passed a key.

11 files changed, 94 insertions, 107 deletions

diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index c30c097b2..a8a7a84d1 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -9,6 +9,7 @@ module Puppet::Indirector
 
     require 'puppet/indirector/indirection'
     require 'puppet/indirector/terminus'
+    require 'puppet/indirector/envelope'
 
     # Declare that the including class indirects its methods to
     # this terminus.  The terminus name must be the name of a Puppet
@@ -20,6 +21,7 @@ module Puppet::Indirector
         # populate this class with the various new methods
         extend ClassMethods
         include InstanceMethods
+        include Puppet::Indirector::Envelope
 
         # instantiate the actual Terminus for that type and this name (:ldap, w/ args :node)
         # & hook the instantiated Terminus into this class (Node: @indirection = terminus)
@@ -28,41 +30,32 @@ module Puppet::Indirector
     end
 
     module ClassMethods   
-      attr_reader :indirection
+        attr_reader :indirection
 
-      def cache_class=(klass)
-          indirection.cache_class = klass
-      end
+        def cache_class=(klass)
+            indirection.cache_class = klass
+        end
 
-      def terminus_class=(klass)
-          indirection.terminus_class = klass
-      end
+        def terminus_class=(klass)
+            indirection.terminus_class = klass
+        end
          
-      def find(*args)
-        indirection.find(*args)
-      end
+        def find(*args)
+            indirection.find Puppet::Indirector::Request.new(indirection.name, :find, *args)
+        end
 
-      def destroy(*args)
-        indirection.destroy(*args)
-      end
+        def destroy(*args)
+            indirection.destroy Puppet::Indirector::Request.new(indirection.name, :destroy, *args)
+        end
 
-      def search(*args)
-        indirection.search(*args)
-      end
-
-      def version(*args)
-        indirection.version(*args)
-      end
+        def search(*args)
+            indirection.search Puppet::Indirector::Request.new(indirection.name, :search, *args)
+        end
     end
 
     module InstanceMethods
-      # Make it easy for the model to set versions,
-      # which are used for caching and such.
-      attr_accessor :version
-
-      # these become instance methods 
-      def save(*args)
-        self.class.indirection.save(self, *args)
-      end
+        def save(*args)
+            self.class.indirection.save Puppet::Indirector::Request.new(self.class.indirection.name, :save, self, *args)
+        end
     end
 end
diff --git a/lib/puppet/indirector/checksum/file.rb b/lib/puppet/indirector/checksum/file.rb
index 3b196a1f8..5489b40e8 100644
--- a/lib/puppet/indirector/checksum/file.rb
+++ b/lib/puppet/indirector/checksum/file.rb
@@ -18,8 +18,8 @@ class Puppet::Checksum::File < Puppet::Indirector::File
         path.join(File::SEPARATOR)
     end
 
-    def save(file)
-        path = File.dirname(path(file.name))
+    def save(request)
+        path = File.dirname(path(request.key))
 
         # Make the directories if necessary.
         unless FileTest.directory?(path)
diff --git a/lib/puppet/indirector/direct_file_server.rb b/lib/puppet/indirector/direct_file_server.rb
index 31cc9aa16..1711356f9 100644
--- a/lib/puppet/indirector/direct_file_server.rb
+++ b/lib/puppet/indirector/direct_file_server.rb
@@ -11,17 +11,17 @@ class Puppet::Indirector::DirectFileServer < Puppet::Indirector::Terminus
     include Puppet::Util::URIHelper
     include Puppet::FileServing::TerminusHelper
 
-    def find(key, options = {})
-        uri = key2uri(key)
+    def find(request)
+        uri = key2uri(request.key)
         return nil unless FileTest.exists?(uri.path)
-        instance = model.new(key, :path => uri.path)
-        instance.links = options[:links] if options[:links]
+        instance = model.new(request.key, :path => uri.path)
+        instance.links = request.options[:links] if request.options[:links]
         return instance
     end
 
-    def search(key, options = {})
-        uri = key2uri(key)
+    def search(request)
+        uri = key2uri(request.key)
         return nil unless FileTest.exists?(uri.path)
-        path2instances(key, uri.path, options)
+        path2instances(request.key, uri.path, request.options)
     end
 end
diff --git a/lib/puppet/indirector/file.rb b/lib/puppet/indirector/file.rb
index 8c984154b..e5382155f 100644
--- a/lib/puppet/indirector/file.rb
+++ b/lib/puppet/indirector/file.rb
@@ -3,27 +3,27 @@ require 'puppet/indirector/terminus'
 # An empty terminus type, meant to just return empty objects.
 class Puppet::Indirector::File < Puppet::Indirector::Terminus
     # Remove files on disk.
-    def destroy(name)
+    def destroy(request)
         if respond_to?(:path)
-            path = path(name)
+            path = path(request.key)
         else
-            path = name
+            path = request.key
         end
-        raise Puppet::Error.new("File %s does not exist; cannot destroy" % [name]) unless File.exist?(path)
+        raise Puppet::Error.new("File %s does not exist; cannot destroy" % [request.key]) unless File.exist?(path)
 
         begin
             File.unlink(path)
         rescue => detail
-            raise Puppet::Error, "Could not remove %s: %s" % [name, detail]
+            raise Puppet::Error, "Could not remove %s: %s" % [request.key, detail]
         end
     end
 
     # Return a model instance for a given file on disk.
-    def find(name)
+    def find(request)
         if respond_to?(:path)
-            path = path(name)
+            path = path(request.key)
         else
-            path = name
+            path = request.key
         end
 
         return nil unless File.exist?(path)
@@ -38,20 +38,20 @@ class Puppet::Indirector::File < Puppet::Indirector::Terminus
     end
 
     # Save a new file to disk.
-    def save(file)
+    def save(request)
         if respond_to?(:path)
-            path = path(file.name)
+            path = path(request.key)
         else
-            path = file.path
+            path = request.key
         end
         dir = File.dirname(path)
 
-        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [file, dir]) unless File.directory?(dir)
+        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [request.key, dir]) unless File.directory?(dir)
 
         begin
-            File.open(path, "w") { |f| f.print file.content }
+            File.open(path, "w") { |f| f.print request.instance.content }
         rescue => detail
-            raise Puppet::Error, "Could not write %s: %s" % [file, detail]
+            raise Puppet::Error, "Could not write %s: %s" % [request.key, detail]
         end
     end
 end
diff --git a/lib/puppet/indirector/file_metadata/file.rb b/lib/puppet/indirector/file_metadata/file.rb
index b36846bbe..c46015c38 100644
--- a/lib/puppet/indirector/file_metadata/file.rb
+++ b/lib/puppet/indirector/file_metadata/file.rb
@@ -9,14 +9,14 @@ require 'puppet/indirector/direct_file_server'
 class Puppet::Indirector::FileMetadata::File < Puppet::Indirector::DirectFileServer
     desc "Retrieve file metadata directly from the local filesystem."
 
-    def find(key, options = {})
+    def find(request)
         return unless data = super
         data.collect_attributes
 
         return data
     end
 
-    def search(key, options = {})
+    def search(request)
         return unless result = super
 
         result.each { |instance| instance.collect_attributes }
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 1b6613035..56cd687af 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -126,6 +126,11 @@ class Puppet::Indirector::Indirection
         end
     end
 
+    # Set up our request object.
+    def request(key, method, arguments = nil)
+        Puppet::Indirector::Request.new(self.name, key, method, arguments)
+    end
+
     # Return the singleton terminus for this indirection.
     def terminus(terminus_name = nil)
         # Get the name of the terminus.
@@ -167,28 +172,27 @@ class Puppet::Indirector::Indirection
         end
     end
 
-    def find(key, *args)
-        request = request(key, :find, *args)
+    def find(request)
         terminus = prepare(request)
 
         # See if our instance is in the cache and up to date.
-        if cache? and cached = cache.find(key, *args)
+        if cache? and cached = cache.find(request)
             if cached.expired?
-                Puppet.info "Cached %s %s expired at %s; not using" % [self.name, key, cached.expiration]
+                Puppet.info "Cached %s %s expired at %s; not using" % [self.name, request.key, cached.expiration]
             else
-                Puppet.debug "Using cached %s %s" % [self.name, key]
+                Puppet.debug "Using cached %s %s" % [self.name, request.key]
                 return cached
             end
         end
 
         # Otherwise, return the result from the terminus, caching if appropriate.
-        if result = terminus.find(key, *args)
-            # Include the envelope module, so we can set the expiration.
-            result.extend(Puppet::Indirector::Envelope)
+        if result = terminus.find(request)
             result.expiration ||= self.expiration
             if cache?
-                Puppet.info "Caching %s %s" % [self.name, key]
-                cache.save(result, *args)
+                Puppet.info "Caching %s %s" % [self.name, request.key]
+                cached_request = request.clone
+                cached_request.instance = result
+                cache.save(cached_request)
             end
 
             return result
@@ -198,38 +202,35 @@ class Puppet::Indirector::Indirection
     end
 
     # Remove something via the terminus.
-    def destroy(key, *args)
-        request = request(key, :destroy, *args)
+    def destroy(request)
         terminus = prepare(request)
 
-        terminus.destroy(key, *args)
+        terminus.destroy(request)
 
-        if cache? and cached = cache.find(key, *args)
-            cache.destroy(key, *args)
+        if cache? and cached = cache.find(request)
+            cache.destroy(request)
         end
 
         nil
     end
 
     # Search for more than one instance.  Should always return an array.
-    def search(key, *args)
-        request = request(key, :search, *args)
+    def search(request)
         terminus = prepare(request)
 
-        result = terminus.search(key, *args)
+        result = terminus.search(request)
 
         result
     end
 
     # Save the instance in the appropriate terminus.  This method is
     # normally an instance method on the indirected class.
-    def save(instance, *args)
-        request = request(instance.name, :save, *args)
+    def save(request)
         terminus = prepare(request)
 
         # If caching is enabled, save our document there
-        cache.save(instance, *args) if cache?
-        terminus.save(instance, *args)
+        cache.save(request) if cache?
+        terminus.save(request)
     end
 
     private
@@ -268,9 +269,4 @@ class Puppet::Indirector::Indirection
         end
         return klass.new
     end
-
-    # Set up our request object.
-    def request(key, method, arguments = nil)
-        Puppet::Indirector::Request.new(self.name, key, method, arguments)
-    end
 end
diff --git a/lib/puppet/indirector/memory.rb b/lib/puppet/indirector/memory.rb
index b97e6ffb6..19acc14e2 100644
--- a/lib/puppet/indirector/memory.rb
+++ b/lib/puppet/indirector/memory.rb
@@ -6,16 +6,16 @@ class Puppet::Indirector::Memory < Puppet::Indirector::Terminus
         @instances = {}
     end
 
-    def destroy(name)
-        raise ArgumentError.new("Could not find %s to destroy" % name) unless @instances.include?(name)
-        @instances.delete(name)
+    def destroy(request)
+        raise ArgumentError.new("Could not find %s to destroy" % request.key) unless @instances.include?(request.key)
+        @instances.delete(request.key)
     end
 
-    def find(name)
-        @instances[name]
+    def find(request)
+        @instances[request.key]
     end
 
-    def save(instance)
-        @instances[instance.name] = instance
+    def save(request)
+        @instances[request.key] = request.instance
     end
 end
diff --git a/lib/puppet/indirector/plain.rb b/lib/puppet/indirector/plain.rb
index 8bdf8469c..2caa0946d 100644
--- a/lib/puppet/indirector/plain.rb
+++ b/lib/puppet/indirector/plain.rb
@@ -3,7 +3,7 @@ require 'puppet/indirector/terminus'
 # An empty terminus type, meant to just return empty objects.
 class Puppet::Indirector::Plain < Puppet::Indirector::Terminus
     # Just return nothing.
-    def find(name)
-        indirection.model.new(name)
+    def find(request)
+        indirection.model.new(request.key)
     end
 end
diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
index 708862b28..68b7ee160 100644
--- a/lib/puppet/indirector/request.rb
+++ b/lib/puppet/indirector/request.rb
@@ -5,7 +5,7 @@ require 'puppet/indirector'
 class Puppet::Indirector::Request
     attr_accessor :indirection_name, :key, :method, :options, :instance
 
-    def initialize(indirection_name, key, method, options = {})
+    def initialize(indirection_name, method, key, options = {})
         @indirection_name, @method, @options = indirection_name, method, (options || {})
 
         if key.is_a?(String) or key.is_a?(Symbol)
diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb
index 4dd29159e..23bca02b8 100644
--- a/lib/puppet/indirector/yaml.rb
+++ b/lib/puppet/indirector/yaml.rb
@@ -3,23 +3,22 @@ require 'puppet/indirector/terminus'
 # The base class for YAML indirection termini.
 class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
     # Read a given name's file in and convert it from YAML.
-    def find(name)
-        raise ArgumentError.new("You must specify the name of the object to retrieve") unless name
-        file = path(name)
+    def find(request)
+        file = path(request.key)
         return nil unless FileTest.exist?(file)
 
         begin
             return from_yaml(File.read(file))
         rescue => detail
-            raise Puppet::Error, "Could not read YAML data for %s %s: %s" % [indirection.name, name, detail]
+            raise Puppet::Error, "Could not read YAML data for %s %s: %s" % [indirection.name, request.key, detail]
         end
     end
 
     # Convert our object to YAML and store it to the disk.
-    def save(object)
-        raise ArgumentError.new("You can only save objects that respond to :name") unless object.respond_to?(:name)
+    def save(request)
+        raise ArgumentError.new("You can only save objects that respond to :name") unless request.instance.respond_to?(:name)
 
-        file = path(object.name)
+        file = path(request.key)
 
         basedir = File.dirname(file)
 
@@ -29,15 +28,15 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
         end
 
         begin
-            File.open(file, "w", 0660) { |f| f.print to_yaml(object) }
+            File.open(file, "w", 0660) { |f| f.print to_yaml(request.instance) }
         rescue TypeError => detail
-            Puppet.err "Could not save %s %s: %s" % [self.name, object.name, detail]
+            Puppet.err "Could not save %s %s: %s" % [self.name, request.key, detail]
         end
     end
 
-    def version(name)
-        return nil unless FileTest.exist?(path(name))
-        return File.stat(path(name)).mtime
+    # Return the path to a given node's file.
+    def path(name)
+        File.join(Puppet[:yamldir], self.class.indirection_name.to_s, name.to_s + ".yaml")
     end
 
     private
@@ -49,9 +48,4 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
     def to_yaml(object)
         YAML.dump(object)
     end
-
-    # Return the path to a given node's file.
-    def path(name)
-        File.join(Puppet[:yamldir], self.class.indirection_name.to_s, name.to_s + ".yaml")
-    end
 end
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 56f8a602a..bd62ebbe6 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -34,6 +34,10 @@ class Puppet::Transaction::Report
         end
     end
 
+    def name
+        host
+    end
+
     # Create a new metric.
     def newmetric(name, hash)
         metric = Puppet::Util::Metric.new(name)