File serving should work now, both recursive and

single files, across modules, local file system,
and the traditional file server.

This work revolves around making sure that the termini
produce functional file instances, meaning that they
know how to find their content or metadata, which largely
comes down to setting their paths correctly.

I also created a new terminus base class for the local
filesystem, since there was so much common code between
content and metadata.

12 files changed, 97 insertions, 55 deletions

diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb
index 063192d15..9398513e7 100644
--- a/lib/puppet/file_serving/content.rb
+++ b/lib/puppet/file_serving/content.rb
@@ -17,11 +17,11 @@ class Puppet::FileServing::Content < Puppet::FileServing::FileBase
     attr_reader :path
 
     # Read the content of our file in.
-    def content(base = nil)
+    def content
         # This stat can raise an exception, too.
-        raise(ArgumentError, "Cannot read the contents of links unless following links") if stat(base).ftype == "symlink" 
+        raise(ArgumentError, "Cannot read the contents of links unless following links") if stat().ftype == "symlink" 
 
-        ::File.read(full_path(base))
+        ::File.read(full_path())
     end
 
     # Just return the file contents as the yaml.  This allows us to
diff --git a/lib/puppet/file_serving/file_base.rb b/lib/puppet/file_serving/file_base.rb
index b2e9a0656..7f169d1ea 100644
--- a/lib/puppet/file_serving/file_base.rb
+++ b/lib/puppet/file_serving/file_base.rb
@@ -7,18 +7,19 @@ require 'puppet/file_serving'
 # The base class for Content and Metadata; provides common
 # functionality like the behaviour around links.
 class Puppet::FileServing::FileBase
-    attr_accessor :path, :base_path
+    attr_accessor :key
 
-    def full_path(base = nil)
-        base ||= base_path || raise(ArgumentError, "You must set or provide a base path")
+    # Return the full path to our file.  Fails if there's no path set.
+    def full_path
+        raise(ArgumentError, "You must set a path to get a file's path") unless self.path
 
-        full = File.join(base, self.path)
+        relative_path ? File.join(path, relative_path) : path
     end
 
-    def initialize(path, options = {})
+    def initialize(key, options = {})
         raise ArgumentError.new("Files must not be fully qualified") if path =~ /^#{::File::SEPARATOR}/
 
-        @path = path
+        @key = key
         @links = :manage
 
         options.each do |param, value|
@@ -30,17 +31,33 @@ class Puppet::FileServing::FileBase
         end
     end
 
+    # Determine how we deal with links.
     attr_reader :links
     def links=(value)
         raise(ArgumentError, ":links can only be set to :manage or :follow") unless [:manage, :follow].include?(value) 
         @links = value
     end
 
+    # Set our base path.
+    attr_reader :path
+    def path=(path)
+        raise ArgumentError.new("Paths must be fully qualified") unless path =~ /^#{::File::SEPARATOR}/
+        @path = path
+    end
+
+    # Set a relative path; this is used for recursion, and sets
+    # the file's path relative to the initial recursion point.
+    attr_reader :relative_path
+    def relative_path=(path)
+        raise ArgumentError.new("Relative paths must not be fully qualified") if path =~ /^#{::File::SEPARATOR}/
+        @relative_path = path
+    end
+
     # Stat our file, using the appropriate link-sensitive method.
-    def stat(base = nil)
+    def stat
         unless defined?(@stat_method)
             @stat_method = self.links == :manage ? :lstat : :stat
         end
-        File.send(@stat_method, full_path(base))
+        File.send(@stat_method, full_path())
     end
 end
diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb
index 410655731..e26e75844 100644
--- a/lib/puppet/file_serving/metadata.rb
+++ b/lib/puppet/file_serving/metadata.rb
@@ -11,6 +11,16 @@ require 'puppet/file_serving/indirection_hooks'
 
 # A class that handles retrieving file metadata.
 class Puppet::FileServing::Metadata < Puppet::FileServing::FileBase
+    module MetadataHelper
+        include Puppet::FileServing::IndirectionHooks
+
+        def post_find(instance)
+        end
+
+        def post_search(key, options = {})
+        end
+    end
+
     include Puppet::Util::Checksums
 
     extend Puppet::Indirector
@@ -27,9 +37,9 @@ class Puppet::FileServing::Metadata < Puppet::FileServing::FileBase
     # Retrieve the attributes for this file, relative to a base directory.
     # Note that File.stat raises Errno::ENOENT if the file is absent and this
     # method does not catch that exception.
-    def collect_attributes(base = nil)
-        real_path = full_path(base)
-        stat = stat(base)
+    def collect_attributes
+        real_path = full_path()
+        stat = stat()
         @owner = stat.uid
         @group = stat.gid
         @ftype = stat.ftype
diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/file_serving/terminus_helper.rb
index 9542cbf84..d465aa493 100644
--- a/lib/puppet/file_serving/terminus_helper.rb
+++ b/lib/puppet/file_serving/terminus_helper.rb
@@ -8,8 +8,12 @@ require 'puppet/file_serving/fileset'
 # Define some common methods for FileServing termini.
 module Puppet::FileServing::TerminusHelper
     # Create model instances for all files in a fileset.
-    def path2instances(path, options = {})
+    def path2instances(key, path, options = {})
         args = [:links, :ignore, :recurse].inject({}) { |hash, param| hash[param] = options[param] if options[param]; hash }
-        Puppet::FileServing::Fileset.new(path, args).files.collect { |file| model.new(file) }
+        Puppet::FileServing::Fileset.new(path, args).files.collect do |file|
+            inst = model.new(File.join(key, file), :path => path, :relative_path => file)
+            inst.links = options[:links] if options[:links]
+            inst
+        end
     end
 end
diff --git a/lib/puppet/indirector/direct_file_server.rb b/lib/puppet/indirector/direct_file_server.rb
new file mode 100644
index 000000000..31cc9aa16
--- /dev/null
+++ b/lib/puppet/indirector/direct_file_server.rb
@@ -0,0 +1,27 @@
+#
+#  Created by Luke Kanies on 2007-10-24.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'puppet/file_serving/terminus_helper'
+require 'puppet/util/uri_helper'
+require 'puppet/indirector/terminus'
+
+class Puppet::Indirector::DirectFileServer < Puppet::Indirector::Terminus
+
+    include Puppet::Util::URIHelper
+    include Puppet::FileServing::TerminusHelper
+
+    def find(key, options = {})
+        uri = key2uri(key)
+        return nil unless FileTest.exists?(uri.path)
+        instance = model.new(key, :path => uri.path)
+        instance.links = options[:links] if options[:links]
+        return instance
+    end
+
+    def search(key, options = {})
+        uri = key2uri(key)
+        return nil unless FileTest.exists?(uri.path)
+        path2instances(key, uri.path, options)
+    end
+end
diff --git a/lib/puppet/indirector/file_content/file.rb b/lib/puppet/indirector/file_content/file.rb
index 4503a7919..30c79583c 100644
--- a/lib/puppet/indirector/file_content/file.rb
+++ b/lib/puppet/indirector/file_content/file.rb
@@ -3,26 +3,9 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'puppet/file_serving/content'
-require 'puppet/file_serving/terminus_helper'
-require 'puppet/util/uri_helper'
 require 'puppet/indirector/file_content'
-require 'puppet/indirector/file'
+require 'puppet/indirector/direct_file_server'
 
-class Puppet::Indirector::FileContent::File < Puppet::Indirector::File
+class Puppet::Indirector::FileContent::File < Puppet::Indirector::DirectFileServer
     desc "Retrieve file contents from disk."
-
-    include Puppet::Util::URIHelper
-    include Puppet::FileServing::TerminusHelper
-
-    def find(key, options = {})
-        uri = key2uri(key)
-        return nil unless FileTest.exists?(uri.path)
-        model.new(uri.path, :links => options[:links])
-    end
-
-    def search(key, options = {})
-        uri = key2uri(key)
-        return nil unless FileTest.exists?(uri.path)
-        path2instances(uri.path, options)
-    end
 end
diff --git a/lib/puppet/indirector/file_metadata/file.rb b/lib/puppet/indirector/file_metadata/file.rb
index 823c26c36..b36846bbe 100644
--- a/lib/puppet/indirector/file_metadata/file.rb
+++ b/lib/puppet/indirector/file_metadata/file.rb
@@ -3,30 +3,24 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'puppet/file_serving/metadata'
-require 'puppet/file_serving/terminus_helper'
 require 'puppet/indirector/file_metadata'
-require 'puppet/util/uri_helper'
-require 'puppet/indirector/code'
+require 'puppet/indirector/direct_file_server'
 
-class Puppet::Indirector::FileMetadata::File < Puppet::Indirector::Code
+class Puppet::Indirector::FileMetadata::File < Puppet::Indirector::DirectFileServer
     desc "Retrieve file metadata directly from the local filesystem."
 
-    include Puppet::Util::URIHelper
-    include Puppet::FileServing::TerminusHelper
-
     def find(key, options = {})
-        uri = key2uri(key)
-
-        return nil unless FileTest.exists?(uri.path)
-        data = model.new(uri.path, :links => options[:links])
+        return unless data = super
         data.collect_attributes
 
         return data
     end
 
     def search(key, options = {})
-        uri = key2uri(key)
-        return nil unless FileTest.exists?(uri.path)
-        path2instances(uri.path, options).each { |instance| instance.collect_attributes }
+        return unless result = super
+
+        result.each { |instance| instance.collect_attributes }
+
+        return result
     end
 end
diff --git a/lib/puppet/indirector/file_metadata/modules.rb b/lib/puppet/indirector/file_metadata/modules.rb
index 739c40fca..5ed7a8a45 100644
--- a/lib/puppet/indirector/file_metadata/modules.rb
+++ b/lib/puppet/indirector/file_metadata/modules.rb
@@ -11,7 +11,7 @@ class Puppet::Indirector::FileMetadata::Modules < Puppet::Indirector::ModuleFile
 
     def find(*args)
         return unless instance = super
-        instance.get_attributes
+        instance.collect_attributes
         instance
     end
 end
diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb
index de88bdc18..2eb323d46 100644
--- a/lib/puppet/indirector/file_server.rb
+++ b/lib/puppet/indirector/file_server.rb
@@ -25,7 +25,9 @@ class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus
     # Find our key using the fileserver.
     def find(key, options = {})
         return nil unless path = find_path(key, options)
-        return model.new(path, :links => options[:links])
+        result =  model.new(key, :path => path)
+        result.links = options[:links] if options[:links]
+        return result
     end
 
     # Search for files.  This returns an array rather than a single
@@ -33,7 +35,7 @@ class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus
     def search(key, options = {})
         return nil unless path = find_path(key, options)
 
-        path2instances(path, options)
+        path2instances(key, path, options)
     end
 
     private
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 60278be98..9a9b1c0bf 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -168,6 +168,10 @@ class Puppet::Indirector::Indirection
     # Check authorization if there's a hook available; fail if there is one
     # and it returns false.
     def check_authorization(method, terminus_name, arguments)
+        # Don't check authorization if there's no node.
+        # LAK:FIXME This is a hack and is quite possibly not the design we want.
+        return unless arguments[-1].is_a?(Hash) and arguments[-1][:node]
+
         if terminus(terminus_name).respond_to?(:authorized?) and ! terminus(terminus_name).authorized?(method, *arguments)
             raise ArgumentError, "Not authorized to call %s with %s" % [method, arguments[0]]
         end
diff --git a/lib/puppet/indirector/module_files.rb b/lib/puppet/indirector/module_files.rb
index 12794e4c7..c79fae57b 100644
--- a/lib/puppet/indirector/module_files.rb
+++ b/lib/puppet/indirector/module_files.rb
@@ -30,7 +30,9 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
     def find(key, options = {})
         return nil unless path = find_path(key, options)
 
-        return model.new(path, :links => options[:links])
+        result = model.new(key, :path => path)
+        result.links = options[:links] if options[:links]
+        return result
     end
 
     # Try to find our module.
@@ -41,7 +43,7 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
     # Search for a list of files.
     def search(key, options = {})
         return nil unless path = find_path(key, options)
-        path2instances(path, options)
+        path2instances(key, path, options)
     end
 
     private
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 2b3df1ae7..f0a805525 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -618,7 +618,6 @@ module Puppet
             # than this last bit, so it doesn't really make sense.
             if child = configuration.resource(:file, path)
                 unless child.parent.object_id == self.object_id
-                    puts("Parent is %s, I am %s" % [child.parent.ref, self.ref]) if child.parent
                     self.debug "Not managing more explicit file %s" %
                         path
                     return nil