I'm working on making file serving work in the indirector now, so I

added two abilities to the indirections:  Models can specify a module to
extend the indirection instance with, and indirections will use a
:select_terminus method, if it's available, to select the terminus to
use for finding.  (It's currently only used for finding, not destroying
or saving.)

The upshot is that a model can have a module that handles terminus
selection for it, and then extend its indirection with that module.

This will allow me to use the local terminus when the protocol is 'file'
and the REST terminus when the protocol is 'puppet'.  It should also
open the door for other protocols if they become available.

11 files changed, 211 insertions, 53 deletions

diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb
index 216072e49..53e52cd2d 100644
--- a/lib/puppet/file_serving/content.rb
+++ b/lib/puppet/file_serving/content.rb
@@ -4,13 +4,14 @@
 
 require 'puppet/indirector'
 require 'puppet/file_serving'
+require 'puppet/file_serving/terminus_selector'
 
 # A class that handles retrieving file contents.
 # It only reads the file when its content is specifically
 # asked for.
 class Puppet::FileServing::Content
     extend Puppet::Indirector
-    indirects :file_content, :terminus_class => :file
+    indirects :file_content, :terminus_class => :file, :extend => Puppet::FileServing::TerminusSelector
 
     attr_reader :path
 
diff --git a/lib/puppet/file_serving/terminus_selector.rb b/lib/puppet/file_serving/terminus_selector.rb
new file mode 100644
index 000000000..0cec4bf98
--- /dev/null
+++ b/lib/puppet/file_serving/terminus_selector.rb
@@ -0,0 +1,26 @@
+#
+#  Created by Luke Kanies on 2007-10-18.
+#  Copyright (c) 2007. All rights reserved.
+
+require 'uri'
+require 'puppet/file_serving'
+
+# This module is used to pick the appropriate terminus
+# in file-serving indirections.  This is necessary because
+# the terminus varies based on the URI asked for.
+module Puppet::FileServing::TerminusSelector
+    PROTOCOL_MAP = {"puppet" => :rest, "file" => :local}
+
+    # Pick an appropriate terminus based on the protocol.
+    def select_terminus(uri)
+        # Short-circuit to :local if it's a fully-qualified path.
+        return PROTOCOL_MAP["file"] if uri =~ /^#{::File::SEPARATOR}/
+        begin
+            uri = URI.parse(URI.escape(uri))
+        rescue => detail
+            raise ArgumentError, "Could not understand URI %s: %s" % [uri, detail.to_s]
+        end
+
+        return PROTOCOL_MAP[uri.scheme] || raise(ArgumentError, "URI protocol '%s' is not supported for file serving" % uri.scheme)
+    end
+end
diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index 6009a7ba7..c30c097b2 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -23,17 +23,7 @@ module Puppet::Indirector
 
         # instantiate the actual Terminus for that type and this name (:ldap, w/ args :node)
         # & hook the instantiated Terminus into this class (Node: @indirection = terminus)
-        @indirection = Puppet::Indirector::Indirection.new(self, indirection)
-
-        unless options.empty?
-            options.each do |param, value|
-                case param
-                when :terminus_class: @indirection.terminus_class = value
-                else
-                    raise ArgumenError, "Invalid option '%s' to 'indirects'" % param
-                end
-            end
-        end
+        @indirection = Puppet::Indirector::Indirection.new(self, indirection,  options)
         @indirection
     end
 
diff --git a/lib/puppet/indirector/file_content/file.rb b/lib/puppet/indirector/file_content/file.rb
index 2723142af..bb9dc6998 100644
--- a/lib/puppet/indirector/file_content/file.rb
+++ b/lib/puppet/indirector/file_content/file.rb
@@ -8,4 +8,7 @@ require 'puppet/indirector/file'
 
 class Puppet::Indirector::FileContent::File < Puppet::Indirector::File
     desc "Retrieve file contents from disk."
+
+    def find(path)
+    end
 end
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 0d814c5ef..313117b25 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -48,6 +48,19 @@ class Puppet::Indirector::Indirection
     def initialize(model, name, options = {})
         @model = model
         @name = name
+
+        @termini = {}
+        @cache_class = nil
+
+        raise(ArgumentError, "Indirection %s is already defined" % @name) if @@indirections.find { |i| i.name == @name }
+        @@indirections << self
+
+        if mod = options[:extend]
+            extend(mod)
+            options.delete(:extend)
+        end
+
+        # This is currently only used for cache_class and terminus_class.
         options.each do |name, value|
             begin
                 send(name.to_s + "=", value)
@@ -55,11 +68,6 @@ class Puppet::Indirector::Indirection
                 raise ArgumentError, "%s is not a valid Indirection parameter" % name
             end
         end
-        @termini = {}
-        @terminus_types = {}
-        @cache_class = nil
-        raise(ArgumentError, "Indirection %s is already defined" % @name) if @@indirections.find { |i| i.name == @name }
-        @@indirections << self
     end
 
     # Return the singleton terminus for this indirection.
@@ -91,11 +99,24 @@ class Puppet::Indirector::Indirection
     end
 
     def find(key, *args)
-        if cache? and cache.has_most_recent?(key, terminus.version(key))
+        # Select the appropriate terminus if there's a hook
+        # for doing so.  This allows the caller to pass in some kind
+        # of URI that the indirection can use for routing to the appropriate
+        # terminus.
+        if respond_to?(:select_terminus)
+            terminus_name = select_terminus(key)
+        else
+            terminus_name = terminus_class
+        end
+
+        # See if our instance is in the cache and up to date.
+        if cache? and cache.has_most_recent?(key, terminus(terminus_name).version(key))
             Puppet.info "Using cached %s %s" % [self.name, key]
             return cache.find(key, *args)
         end
-        if result = terminus.find(key, *args)
+
+        # Otherwise, return the result from the terminus, caching if appropriate.
+        if result = terminus(terminus_name).find(key, *args)
             result.version ||= Time.now.utc
             if cache?
                 Puppet.info "Caching %s %s" % [self.name, key]
diff --git a/spec/unit/file_serving/content.rb b/spec/unit/file_serving/content.rb
index 08397a067..6d8e2d0ba 100755
--- a/spec/unit/file_serving/content.rb
+++ b/spec/unit/file_serving/content.rb
@@ -8,6 +8,10 @@ describe Puppet::FileServing::Content do
     it "should indirect file_content" do
         Puppet::FileServing::Content.indirection.name.should == :file_content
     end
+
+    it "should should include the TerminusSelector module in its indirection" do
+        Puppet::FileServing::Content.indirection.metaclass.included_modules.should include(Puppet::FileServing::TerminusSelector)
+    end
 end
 
 describe Puppet::FileServing::Content, " when initializing" do
diff --git a/spec/unit/file_serving/terminus_selector.rb b/spec/unit/file_serving/terminus_selector.rb
new file mode 100755
index 000000000..4a5683e88
--- /dev/null
+++ b/spec/unit/file_serving/terminus_selector.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2007-10-18.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/file_serving/terminus_selector'
+
+describe Puppet::FileServing::TerminusSelector, " when being used to select termini" do
+    before do
+        @object = Object.new
+        @object.extend(Puppet::FileServing::TerminusSelector)
+    end
+
+    it "should choose :rest when the protocol is 'puppet'" do
+        @object.select_terminus("puppet://host/module/file").should == :rest
+    end
+
+    it "should choose :local when the protocol is 'file'" do
+        @object.select_terminus("file://host/module/file").should == :local
+    end
+
+    it "should choose :local when the URI is a normal path name" do
+        @object.select_terminus("/module/file").should == :local
+    end
+
+    it "should fail when a protocol other than :puppet or :file is used" do
+        proc { @object.select_terminus("http:///module/file") }.should raise_error(ArgumentError)
+    end
+end
diff --git a/spec/unit/indirector/indirector.rb b/spec/unit/indirector.rb
index 64c7bdfca..1a5867c51 100755
--- a/spec/unit/indirector/indirector.rb
+++ b/spec/unit/indirector.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../spec_helper'
+require File.dirname(__FILE__) + '/../spec_helper'
 
 require 'puppet/defaults'
 require 'puppet/indirector'
@@ -44,11 +44,10 @@ describe Puppet::Indirector, "when registering an indirection" do
         @thingie.indirection.should equal(@indirection)
     end
 
-    it "should allow specification of a default terminus" do
+    it "should pass any provided options to the indirection during initialization" do
         klass = mock 'terminus class'
-        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:first, :foo).returns(klass)
-        @indirection = @thingie.indirects :first, :terminus_class => :foo
-        @indirection.terminus_class.should == :foo
+        Puppet::Indirector::Indirection.expects(:new).with(@thingie, :first, {:some => :options})
+        @indirection = @thingie.indirects :first, :some => :options
     end
 
     after do
diff --git a/spec/unit/indirector/file_content/file.rb b/spec/unit/indirector/file_content/file.rb
index 68f8a38d0..08f66acb6 100755
--- a/spec/unit/indirector/file_content/file.rb
+++ b/spec/unit/indirector/file_content/file.rb
@@ -11,5 +11,19 @@ describe Puppet::Indirector::FileContent::File do
     it "should be registered with the file_content indirection" do
         Puppet::Indirector::Terminus.terminus_class(:file_content, :file).should equal(Puppet::Indirector::FileContent::File)
     end
+
+    it "should be a subclass of the File terminus" do
+        Puppet::Indirector::FileContent::File.superclass.should equal(Puppet::Indirector::File)
+    end
 end
 
+describe Puppet::Indirector::FileContent::File, "when finding a single file" do
+    before do
+        @content = Puppet::Indirector::FileContent::File.new
+        @path = "/my/file"
+    end
+
+    it "should return nil if the file does not exist"
+
+    it "should return a Content instance with the path set to the file if the file exists"
+end
diff --git a/spec/unit/indirector/file_content/rest.rb b/spec/unit/indirector/file_content/rest.rb
index 725e35075..afb674e0a 100755
--- a/spec/unit/indirector/file_content/rest.rb
+++ b/spec/unit/indirector/file_content/rest.rb
@@ -6,4 +6,6 @@ require 'puppet/indirector/file_content'
 
 describe "Puppet::Indirector::Content::Rest" do
     it "should add the node's cert name to the arguments"
+
+    it "should set the content type to text/plain"
 end
diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb
index 9963725da..9b1f556a6 100755
--- a/spec/unit/indirector/indirection.rb
+++ b/spec/unit/indirector/indirection.rb
@@ -19,15 +19,41 @@ module IndirectionTesting
     end
 end
 
-describe Puppet::Indirector::Indirection do
-    include IndirectionTesting
+describe Puppet::Indirector::Indirection, " when initializing" do
+    # LAK:FIXME I've no idea how to test this, really.
+    it "should store a reference to itself before it consumes its options" do
+        proc { @indirection = Puppet::Indirector::Indirection.new(Object.new, :testingness, :not_valid_option) }.should raise_error
+        Puppet::Indirector::Indirection.instance(:testingness).should be_instance_of(Puppet::Indirector::Indirection)
+        Puppet::Indirector::Indirection.instance(:testingness).delete
+    end
 
-    it "should not attempt to set a timestamp if the terminus cannot find the instance" do
-        @terminus.expects(:find).with(@name).returns(nil)
-        proc { @indirection.find(@name) }.should_not raise_error
+    it "should keep a reference to the indirecting model" do
+        model = mock 'model'
+        @indirection = Puppet::Indirector::Indirection.new(model, :myind)
+        @indirection.model.should equal(model)
+    end
+
+    it "should set the name" do
+        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :myind)
+        @indirection.name.should == :myind
+    end
+
+    it "should require indirections to have unique names" do
+        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
+        proc { Puppet::Indirector::Indirection.new(:test) }.should raise_error(ArgumentError)
+    end
+
+    it "should extend itself with any specified module" do
+        mod = Module.new
+        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test, :extend => mod)
+        @indirection.metaclass.included_modules.should include(mod)
+    end
+
+    after do
+        @indirection.delete if defined? @indirection
     end
 end
-  
+
 describe Puppet::Indirector::Indirection, " when looking for a model instance" do
     include IndirectionTesting
 
@@ -36,6 +62,11 @@ describe Puppet::Indirector::Indirection, " when looking for a model instance" d
         @indirection.find(@name).should == @instance
     end
 
+    it "should not attempt to set a timestamp if the terminus cannot find the instance" do
+        @terminus.expects(:find).with(@name).returns(nil)
+        proc { @indirection.find(@name) }.should_not raise_error
+    end
+
     it "should call the terminus's authorization hook if there is one"
 end
 
@@ -110,28 +141,6 @@ describe Puppet::Indirector::Indirection, " when handling instance versions" do
     end
 end
 
-describe Puppet::Indirector::Indirection, " when initializing" do
-    it "should keep a reference to the indirecting model" do
-        model = mock 'model'
-        @indirection = Puppet::Indirector::Indirection.new(model, :myind)
-        @indirection.model.should equal(model)
-    end
-
-    it "should set the name" do
-        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :myind)
-        @indirection.name.should == :myind
-    end
-
-    it "should require indirections to have unique names" do
-        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
-        proc { Puppet::Indirector::Indirection.new(:test) }.should raise_error(ArgumentError)
-    end
-
-    after do
-        @indirection.delete if defined? @indirection
-    end
-end
-
 describe Puppet::Indirector::Indirection, " when managing indirection instances" do
     it "should allow an indirection to be retrieved by name" do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
@@ -147,7 +156,7 @@ describe Puppet::Indirector::Indirection, " when managing indirection instances"
     end
 end
 
-describe Puppet::Indirector::Indirection, " when specifying terminus types" do
+describe Puppet::Indirector::Indirection, " when specifying the terminus class to use" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
         @terminus = mock 'terminus'
@@ -191,6 +200,64 @@ describe Puppet::Indirector::Indirection, " when specifying terminus types" do
     end
 end
 
+describe Puppet::Indirector::Indirection, " when a select_terminus hook is available" do
+    before do
+        @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
+
+        # And provide a select_terminus hook
+        @indirection.meta_def(:select_terminus) do |uri|
+            :other
+        end
+
+        @terminus = mock 'terminus'
+        @terminus_class = stub 'terminus class', :new => @terminus
+
+        @other_terminus = mock 'other_terminus'
+        @other_terminus_class = stub 'other_terminus_class', :new => @other_terminus
+
+        @cache_terminus = mock 'cache_terminus'
+        @cache_terminus_class = stub 'cache_terminus_class', :new => @cache_terminus
+
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:test, :foo).returns(@terminus_class)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:test, :other).returns(@other_terminus_class)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:test, :cache).returns(@cache_terminus_class)
+
+        # Set it to a default type.
+        @indirection.terminus_class = :foo
+
+        @uri = "full://url/path"
+        @result = stub 'result', :version => 1.0
+    end
+
+    it "should use the terminus name provided by passing the key to the :select_terminus hook when finding instances" do
+        # Set up the expectation
+        @other_terminus.expects(:find).with(@uri).returns(@result)
+
+        @indirection.find(@uri)
+    end
+
+    it "should use the terminus name provided by passing the key to the :select_terminus hook when testing if a cached instance is up to date" do
+        @indirection.cache_class = :cache
+
+        @other_terminus.expects(:version).with(@uri).returns(2.0)
+
+        @cache_terminus.expects(:has_most_recent?).with(@uri, 2.0).returns(true)
+        @cache_terminus.expects(:find).returns(:whatever)
+
+        @indirection.find(@uri).should == :whatever
+    end
+
+    it "should pass the original key to the terminus rather than a modified key" do
+        # This is the same test as before
+        @other_terminus.expects(:find).with(@uri).returns(@result)
+        @indirection.find(@uri)
+    end
+
+    after do
+        @indirection.delete if defined? @indirection
+    end
+end
+
 describe Puppet::Indirector::Indirection, " when managing terminus instances" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)