File serving now works. I've tested a couple of ways to

use it, and added integration tests at the most important
hook points.

This provides the final class structure for all of these classes,
but a lot of the class names are pretty bad, so I'm planning on
going through all of them (especially the file_server stuff) and
renaming.

The functionality is all here for finding files, though (finally).

Once the classes are renamed, I'll be adding searching ability
(which will enable the recursive file copies) and then adding
the link management and enabling ignoring files.

16 files changed, 152 insertions, 113 deletions

diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb
index 6cd99b267..03be1b9dd 100644
--- a/lib/puppet/file_serving/configuration.rb
+++ b/lib/puppet/file_serving/configuration.rb
@@ -28,21 +28,14 @@ class Puppet::FileServing::Configuration
 
     private_class_method  :new
 
-    # Return a content instance.
-    def content(path, options = {})
-        mount, file_path = split_path(path, options[:node])
-
-        return nil unless mount
-
-        mount.file_instance :content, file_path, options
-    end
-
     # Search for a file.
     def file_path(key, options = {})
         mount, file_path = split_path(key, options[:node])
 
         return nil unless mount
 
+        # The mount checks to see if the file exists, and returns nil
+        # if not.
         return mount.file(file_path, options)
     end
 
@@ -55,15 +48,6 @@ class Puppet::FileServing::Configuration
         readconfig(false)
     end
 
-    # Return a metadata instance.
-    def metadata(path, options = {})
-        mount, file_path = split_path(path, options[:node])
-
-        return nil unless mount
-
-        mount.file_instance :metadata, file_path, options
-    end
-
     # Is a given mount available?
     def mounted?(name)
         @mounts.include?(name)
@@ -85,27 +69,6 @@ class Puppet::FileServing::Configuration
         return children
     end  
 
-    # Return the mount for the Puppet modules; allows file copying from
-    # the modules.
-    def modules_mount(module_name, client)
-        unless hostname = (client || Facter.value("hostname"))
-            raise ArgumentError, "Could not find hostname"
-        end
-        if node = Puppet::Node.find(hostname)
-            env = node.environment
-        else
-            env = nil
-        end
-
-        # And use the environment to look up the module.
-        mod = Puppet::Module::find(module_name, env)
-        if mod
-            return @mounts[Mount::MODULES].copy(mod.name, mod.files)
-        else
-            return nil
-        end
-    end
-
     # Read the configuration file.
     def readconfig(check = true)
         config = Puppet[:fileserverconfig]
@@ -139,7 +102,7 @@ class Puppet::FileServing::Configuration
         # Strip off the mount name.
         mount_name, path = uri.sub(%r{^/}, '').split(File::Separator, 2)
 
-        return nil unless mount = modules_mount(mount_name, node) || @mounts[mount_name]
+        return nil unless mount = @mounts[mount_name]
 
         if path == ""
             path = nil
diff --git a/lib/puppet/file_serving/configuration/parser.rb b/lib/puppet/file_serving/configuration/parser.rb
index 5b17d9801..707c3f9b1 100644
--- a/lib/puppet/file_serving/configuration/parser.rb
+++ b/lib/puppet/file_serving/configuration/parser.rb
@@ -3,6 +3,7 @@ require 'puppet/util/loadedfile'
 
 class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile
     Mount = Puppet::FileServing::Mount
+    MODULES = 'modules'
 
     # Parse our configuration file.
     def parse
@@ -52,10 +53,10 @@ class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile
 
     # Add the mount for getting files from modules.
     def add_module_mount
-        unless @mounts[Mount::MODULES]
-            mount = Mount.new(Mount::MODULES)
+        unless @mounts[MODULES]
+            mount = Mount.new(MODULES)
             mount.allow("*")
-            @mounts[Mount::MODULES] = mount
+            @mounts[MODULES] = mount
         end
     end
 
@@ -98,8 +99,8 @@ class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile
 
     # Set the path for a mount.
     def path(mount, value)
-        if mount.name == Mount::MODULES
-            Puppet.warning "The '#{Mount::MODULES}' module can not have a path. Ignoring attempt to set it"
+        if mount.name == MODULES
+            Puppet.warning "The '#{MODULES}' module can not have a path. Ignoring attempt to set it"
         else
             begin
                 mount.path = value
diff --git a/lib/puppet/file_serving/mount.rb b/lib/puppet/file_serving/mount.rb
index be67e6d2b..8e5bd03e8 100644
--- a/lib/puppet/file_serving/mount.rb
+++ b/lib/puppet/file_serving/mount.rb
@@ -13,10 +13,12 @@ require 'puppet/file_serving/content'
 class Puppet::FileServing::Mount < Puppet::Network::AuthStore
     include Puppet::Util::Logging
 
-    # A constant that defines how we refer to our modules mount.
-    MODULES = "modules"
+    @@localmap = nil
 
-    InstanceTypes = {:metadata => Puppet::FileServing::Metadata, :content => Puppet::FileServing::Content}
+    # Clear the cache.  This is only ever used for testing.
+    def self.clear_cache
+        @@localmap = nil
+    end
 
     attr_reader :name
 
@@ -30,14 +32,12 @@ class Puppet::FileServing::Mount < Puppet::Network::AuthStore
     end
 
     # Return an instance of the appropriate class.
-    def file_instance(return_type, short_file, options = {})
-        raise(ArgumentError, "Invalid file type %s" % return_type) unless InstanceTypes.include?(return_type)
-
+    def file(short_file, options = {})
         file = file_path(short_file, options[:node])
 
         return nil unless FileTest.exists?(file)
 
-        return InstanceTypes[return_type].new(file)
+        return file
     end
 
     # Return a fully qualified path, given a short path and
@@ -85,11 +85,8 @@ class Puppet::FileServing::Mount < Puppet::Network::AuthStore
             # Mark that we're expandable.
             @expandable = true
         else
-            unless FileTest.exists?(path)
-                raise ArgumentError, "%s does not exist" % path
-            end
             unless FileTest.directory?(path)
-                raise ArgumentError, "%s is not a directory" % path
+                raise ArgumentError, "%s does not exist or is not a directory" % path
             end
             unless FileTest.readable?(path)
                 raise ArgumentError, "%s is not readable" % path
@@ -111,11 +108,7 @@ class Puppet::FileServing::Mount < Puppet::Network::AuthStore
     # Verify our configuration is valid.  This should really check to
     # make sure at least someone will be allowed, but, eh.
     def valid?
-        if name == MODULES
-            return @path.nil?
-        else
-            return ! @path.nil?
-        end
+        return ! @path.nil?
     end
 
     private
@@ -157,6 +150,7 @@ class Puppet::FileServing::Mount < Puppet::Network::AuthStore
             # Else, use the local information
             map = localmap()
         end
+
         path.gsub(/%(.)/) do |v|
             key = $1
             if key == "%" 
@@ -179,7 +173,7 @@ class Puppet::FileServing::Mount < Puppet::Network::AuthStore
     # Cache this manufactured map, since if it's used it's likely
     # to get used a lot.
     def localmap
-        unless defined? @@localmap
+        unless @@localmap
             @@localmap = {
                 "h" =>  Facter.value("hostname"),
                 "H" => [Facter.value("hostname"),
diff --git a/lib/puppet/file_serving/terminus_selector.rb b/lib/puppet/file_serving/terminus_selector.rb
index 4525a8570..08009cd2b 100644
--- a/lib/puppet/file_serving/terminus_selector.rb
+++ b/lib/puppet/file_serving/terminus_selector.rb
@@ -29,6 +29,10 @@ module Puppet::FileServing::TerminusSelector
             terminus = :mounts
         end
 
+        if uri.path =~ /^\/modules\b/ and terminus == :mounts
+            terminus = :modules
+        end
+
         return terminus
     end
 end
diff --git a/lib/puppet/indirector/file_content/local.rb b/lib/puppet/indirector/file_content/local.rb
index a597fea55..e429c6c25 100644
--- a/lib/puppet/indirector/file_content/local.rb
+++ b/lib/puppet/indirector/file_content/local.rb
@@ -3,19 +3,21 @@
 #  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'
 
 class Puppet::Indirector::FileContent::Local < Puppet::Indirector::File
     desc "Retrieve file contents from disk."
 
-    include Puppet::FileServing::TerminusHelper
+    include Puppet::Util::URIHelper
 
-    def find(key)
+    def find(key, options = {})
         uri = key2uri(key)
 
         return nil unless FileTest.exists?(uri.path)
-        Puppet::FileServing::Content.new uri.path
+        data = model.new(uri.path)
+
+        return data
     end
 end
diff --git a/lib/puppet/indirector/file_content/modules.rb b/lib/puppet/indirector/file_content/modules.rb
new file mode 100644
index 000000000..8563dacb4
--- /dev/null
+++ b/lib/puppet/indirector/file_content/modules.rb
@@ -0,0 +1,11 @@
+#
+#  Created by Luke Kanies on 2007-10-18.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'puppet/file_serving/content'
+require 'puppet/indirector/file_content'
+require 'puppet/indirector/module_files'
+
+class Puppet::Indirector::FileContent::Modules < Puppet::Indirector::ModuleFiles
+    desc "Retrieve file contents from modules."
+end
diff --git a/lib/puppet/indirector/file_content/mounts.rb b/lib/puppet/indirector/file_content/mounts.rb
index 3d147d65a..b11fc628c 100644
--- a/lib/puppet/indirector/file_content/mounts.rb
+++ b/lib/puppet/indirector/file_content/mounts.rb
@@ -3,26 +3,9 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'puppet/file_serving/content'
-require 'puppet/file_serving/terminus_helper'
 require 'puppet/indirector/file_content'
-require 'puppet/indirector/code'
+require 'puppet/indirector/file_server'
 
-class Puppet::Indirector::FileContent::Mounts < Puppet::Indirector::Code
+class Puppet::Indirector::FileContent::Mounts < Puppet::Indirector::FileServer
     desc "Retrieve file contents using Puppet's fileserver."
-
-    include Puppet::FileServing::TerminusHelper
-
-    # This way it can be cleared or whatever and we aren't retaining
-    # a reference to the old one.
-    def configuration
-        Puppet::FileServing::Configuration.create
-    end
-
-    def find(key)
-        uri = key2uri(key)
-
-        return nil unless path = configuration.file_path(uri.path) and FileTest.exists?(path)
-
-        Puppet::FileServing::Content.new path
-    end
 end
diff --git a/lib/puppet/indirector/file_content/rest.rb b/lib/puppet/indirector/file_content/rest.rb
index 9e2de360c..31df7626d 100644
--- a/lib/puppet/indirector/file_content/rest.rb
+++ b/lib/puppet/indirector/file_content/rest.rb
@@ -3,7 +3,7 @@
 #  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/rest'
 
diff --git a/lib/puppet/indirector/file_metadata/local.rb b/lib/puppet/indirector/file_metadata/local.rb
index e1d774cc8..f40d4ce43 100644
--- a/lib/puppet/indirector/file_metadata/local.rb
+++ b/lib/puppet/indirector/file_metadata/local.rb
@@ -4,19 +4,19 @@
 
 require 'puppet/file_serving/metadata'
 require 'puppet/indirector/file_metadata'
-require 'puppet/file_serving/terminus_helper'
+require 'puppet/util/uri_helper'
 require 'puppet/indirector/code'
 
 class Puppet::Indirector::FileMetadata::Local < Puppet::Indirector::Code
     desc "Retrieve file metadata directly from the local filesystem."
 
-    include Puppet::FileServing::TerminusHelper
+    include Puppet::Util::URIHelper
 
     def find(key)
         uri = key2uri(key)
 
         return nil unless FileTest.exists?(uri.path)
-        data = Puppet::FileServing::Metadata.new uri.path
+        data = model.new(uri.path)
         data.get_attributes
 
         return data
diff --git a/lib/puppet/indirector/file_metadata/modules.rb b/lib/puppet/indirector/file_metadata/modules.rb
new file mode 100644
index 000000000..739c40fca
--- /dev/null
+++ b/lib/puppet/indirector/file_metadata/modules.rb
@@ -0,0 +1,17 @@
+#
+#  Created by Luke Kanies on 2007-10-18.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'puppet/file_serving/metadata'
+require 'puppet/indirector/file_metadata'
+require 'puppet/indirector/module_files'
+
+class Puppet::Indirector::FileMetadata::Modules < Puppet::Indirector::ModuleFiles
+    desc "Retrieve file metadata from modules."
+
+    def find(*args)
+        return unless instance = super
+        instance.get_attributes
+        instance
+    end
+end
diff --git a/lib/puppet/indirector/file_metadata/mounts.rb b/lib/puppet/indirector/file_metadata/mounts.rb
index 6d7fe15c6..b1e3b32fd 100644
--- a/lib/puppet/indirector/file_metadata/mounts.rb
+++ b/lib/puppet/indirector/file_metadata/mounts.rb
@@ -3,26 +3,9 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'puppet/file_serving/metadata'
-require 'puppet/file_serving/terminus_helper'
 require 'puppet/indirector/file_metadata'
-require 'puppet/indirector/code'
+require 'puppet/indirector/file_server'
 
-class Puppet::Indirector::FileMetadata::Mounts < Puppet::Indirector::Code
+class Puppet::Indirector::FileMetadata::Mounts < Puppet::Indirector::FileServer
     desc "Retrieve file metadata using Puppet's fileserver."
-
-    include Puppet::FileServing::TerminusHelper
-
-    # This way it can be cleared or whatever and we aren't retaining
-    # a reference to the old one.
-    def configuration
-        Puppet::FileServing::Configuration.create
-    end
-
-    def find(key)
-        uri = key2uri(key)
-
-        return nil unless path = configuration.file_path(uri.path) and FileTest.exists?(path)
-
-        Puppet::FileServing::Metadata.new path
-    end
 end
diff --git a/lib/puppet/indirector/file_metadata/rest.rb b/lib/puppet/indirector/file_metadata/rest.rb
index dcf875b25..0f3d9c6fd 100644
--- a/lib/puppet/indirector/file_metadata/rest.rb
+++ b/lib/puppet/indirector/file_metadata/rest.rb
@@ -3,7 +3,7 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'puppet/file_serving/metadata'
-require 'puppet/file_serving/terminus_helper'
+require 'puppet/util/uri_helper'
 require 'puppet/indirector/file_metadata'
 require 'puppet/indirector/rest'
 
diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb
new file mode 100644
index 000000000..1b2e047e8
--- /dev/null
+++ b/lib/puppet/indirector/file_server.rb
@@ -0,0 +1,34 @@
+#
+#  Created by Luke Kanies on 2007-10-19.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'puppet/util/uri_helper'
+require 'puppet/file_serving/configuration'
+require 'puppet/indirector/terminus'
+
+# Look files up using the file server.
+class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus
+    include Puppet::Util::URIHelper
+
+    # Find our key using the fileserver.
+    def find(key, options = {})
+        uri = key2uri(key)
+
+        # First try the modules mount, at least for now.
+        if instance = indirection.terminus(:modules).find(key, options)
+            Puppet.warning "DEPRECATION NOTICE: Found file in module without using the 'modules' mount; please fix"
+            return instance
+        end
+
+        return nil unless path = configuration.file_path(uri.path, :node => options[:node]) and FileTest.exists?(path)
+
+        return model.new(path)
+    end
+
+    private
+
+    # Our fileserver configuration, if needed.
+    def configuration
+        Puppet::FileServing::Configuration.create
+    end
+end
diff --git a/lib/puppet/indirector/module_files.rb b/lib/puppet/indirector/module_files.rb
new file mode 100644
index 000000000..e0374d7a4
--- /dev/null
+++ b/lib/puppet/indirector/module_files.rb
@@ -0,0 +1,47 @@
+#
+#  Created by Luke Kanies on 2007-10-19.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'puppet/util/uri_helper'
+require 'puppet/indirector/terminus'
+
+# Look files up in Puppet modules.
+class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
+    include Puppet::Util::URIHelper
+
+    # Find our key in a module.
+    def find(key, options = {})
+        uri = key2uri(key)
+
+        # Strip off /modules if it's there -- that's how requests get routed to this terminus.
+        # Also, strip off the leading slash if present.
+        module_name, relative_path = uri.path.sub(/^\/modules\b/, '').sub(%r{^/}, '').split(File::Separator, 2)
+
+        # And use the environment to look up the module.
+        return nil unless mod = find_module(module_name, options[:node])
+
+        path = File.join(mod.files, relative_path)
+
+        return nil unless FileTest.exists?(path)
+
+        return model.new(path)
+    end
+
+    private
+    
+    # Determine the environment to use, if any.
+    def environment(node_name)
+        if node_name and node = Puppet::Node.find(node_name)
+            node.environment
+        elsif env = Puppet.settings[:environment] and env != ""
+            env
+        else
+            nil
+        end
+    end
+
+    # Try to find our module.
+    def find_module(module_name, node_name)
+        Puppet::Module::find(module_name, environment(node_name))
+    end
+end
diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index 8d51aff09..7b7c932c4 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -2,7 +2,7 @@ require 'puppet/indirector/rest'
 
 # Access objects via REST
 class Puppet::Indirector::REST < Puppet::Indirector::Terminus
-    def find(name)
+    def find(name, options = {})
         indirection.model.new(name)
     end
 end
diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/util/uri_helper.rb
index b7f560c57..cb9320387 100644
--- a/lib/puppet/file_serving/terminus_helper.rb
+++ b/lib/puppet/util/uri_helper.rb
@@ -3,10 +3,10 @@
 #  Copyright (c) 2007. All rights reserved.
 
 require 'uri'
-require 'puppet/file_serving'
-require 'puppet/file_serving/configuration'
+require 'puppet/util'
 
-module Puppet::FileServing::TerminusHelper
+# Helper methods for dealing with URIs.
+module Puppet::Util::URIHelper
     def key2uri(key)
         # Return it directly if it's fully qualified.
         if key =~ /^#{::File::SEPARATOR}/