Switching the indirection from using settings for configuration

to requiring explicit configuration.  This means that if
you as an application developer want to use a different indirection
terminus then you have to specify it; something like:

    Puppet::Node.terminus_class = :ldap

Caches use the same kind of configuration:

    Puppet::Node.cache_class = :memory

Accordingly, I've removed the existing setting definitions
from the defaults.rb.

3 files changed, 88 insertions, 90 deletions

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 1b0b402ec..6ea4eef4c 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -500,15 +500,6 @@ module Puppet
             "The server through which to send email reports."]
     )
 
-    # This needs to be in main because it's used too early in the system, such that
-    # we get an infinite loop otherwise.
-    self.setdefaults(:main,
-        :facts_terminus => ["yaml",
-            "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(:rails,
         :dblocation => { :default => "$statedir/clientconfigs.sqlite3",
             :mode => 0660,
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 9cc116e40..3a6284877 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -17,34 +17,26 @@ class Puppet::Indirector::Indirection
 
     # Create and return our cache terminus.
     def cache
-        terminus(cache_name)
+        raise(Puppet::DevError, "Tried to cache when no cache class was set") unless cache_class
+        terminus(cache_class)
     end
 
     # Should we use a cache?
     def cache?
-        cache_name ? true : false
-    end
-
-    # Figure out the cache name, if there is one.  If there's no name, then
-    # caching is disabled.
-    def cache_name
-        unless @cache_name
-            setting_name = "%s_cache" % self.name
-            if ! Puppet.settings.valid?(setting_name) or (value = Puppet.settings[setting_name] and value == "none")
-                @cache_name = nil
-            else
-                @cache_name = value
-                @cache_name = @cache_name.intern if @cache_name.is_a?(String)
-            end
-        end
-        @cache_name
+        cache_class ? true : false
+    end
+
+    attr_reader :cache_class
+    # Define a terminus class to be used for caching.
+    def cache_class=(class_name)
+        validate_terminus_class(class_name)
+        @cache_class = class_name
     end
 
     # Clear our cached list of termini, and reset the cache name
     # so it's looked up again.
     # This is only used for testing.
     def clear_cache
-        @cache_name = nil
         @termini.clear
     end
 
@@ -65,7 +57,7 @@ class Puppet::Indirector::Indirection
         end
         @termini = {}
         @terminus_types = {}
-        @cache_name = nil
+        @cache_class = nil
         raise(ArgumentError, "Indirection %s is already defined" % @name) if @@indirections.find { |i| i.name == @name }
         @@indirections << self
     end
@@ -73,22 +65,31 @@ class Puppet::Indirector::Indirection
     # Return the singleton terminus for this indirection.
     def terminus(terminus_name = nil)
         # Get the name of the terminus.
-        unless terminus_name
-            param_name = "%s_terminus" % self.name
-            if Puppet.settings.valid?(param_name)
-                terminus_name = Puppet.settings[param_name]
-            else
-                terminus_name = Puppet[:default_terminus]
-            end
-            unless terminus_name and terminus_name.to_s != ""
-                raise ArgumentError, "Invalid terminus name %s" % terminus_name.inspect
-            end
-            terminus_name = terminus_name.intern if terminus_name.is_a?(String)
+        unless terminus_name ||= terminus_class
+            raise Puppet::DevError, "No terminus specified for %s; cannot redirect" % self.name
         end
         
         return @termini[terminus_name] ||= make_terminus(terminus_name)
     end
 
+    attr_reader :terminus_class
+
+    # Specify the terminus class to use.
+    def terminus_class=(terminus_class)
+        validate_terminus_class(terminus_class)
+        @terminus_class = terminus_class
+    end
+
+    # This is used by terminus_class= and cache=.
+    def validate_terminus_class(terminus_class)
+        unless terminus_class and terminus_class.to_s != ""
+            raise ArgumentError, "Invalid terminus name %s" % terminus_class.inspect
+        end
+        unless Puppet::Indirector::Terminus.terminus_class(terminus_class, self.name)
+            raise ArgumentError, "Could not find terminus %s for indirection %s" % [terminus_class, self.name]
+        end
+    end
+
     def find(*args)
         terminus.find(*args)
     end
@@ -111,10 +112,10 @@ class Puppet::Indirector::Indirection
     private
 
     # Create a new terminus instance.
-    def make_terminus(name)
+    def make_terminus(terminus_class)
         # Load our terminus class.
-        unless klass = Puppet::Indirector::Terminus.terminus_class(name, self.name)
-            raise ArgumentError, "Could not find terminus %s for indirection %s" % [name, self.name]
+        unless klass = Puppet::Indirector::Terminus.terminus_class(terminus_class, self.name)
+            raise ArgumentError, "Could not find terminus %s for indirection %s" % [terminus_class, self.name]
         end
         return klass.new
     end
diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb
index 41f50522f..4311c88bf 100755
--- a/spec/unit/indirector/indirection.rb
+++ b/spec/unit/indirector/indirection.rb
@@ -74,44 +74,43 @@ describe Puppet::Indirector::Indirection, " when managing indirection instances"
     end
 end
 
-describe Puppet::Indirector::Indirection, " when choosing terminus types" do
+describe Puppet::Indirector::Indirection, " when specifying terminus types" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
         @terminus = mock 'terminus'
         @terminus_class = stub 'terminus class', :new => @terminus
     end
 
-    it "should follow a convention on using per-model configuration parameters to determine the terminus class" do
-        Puppet.settings.expects(:valid?).with('test_terminus').returns(true)
-        Puppet.settings.expects(:value).with('test_terminus').returns(:foo)
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus.should equal(@terminus)
-    end
-
-    it "should use a default system-wide configuration parameter parameter to determine the terminus class when no
-    per-model configuration parameter is available" do
-        Puppet.settings.expects(:valid?).with('test_terminus').returns(false)
-        Puppet.settings.expects(:value).with(:default_terminus).returns(:foo)
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus.should equal(@terminus)
+    it "should allow specification of a terminus type" do
+        @indirection.should respond_to(:terminus_class=)
     end
 
-    it "should select the specified terminus class if a name is provided" do
-        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
-        @indirection.terminus(:foo).should equal(@terminus)
+    it "should fail to redirect if no terminus type has been specified" do
+        proc { @indirection.find("blah") }.should raise_error(Puppet::DevError)
     end
 
     it "should fail when the terminus class name is an empty string" do
-        proc { @indirection.terminus("") }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = "" }.should raise_error(ArgumentError)
     end
 
     it "should fail when the terminus class name is nil" do
-        proc { @indirection.terminus(nil) }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = nil }.should raise_error(ArgumentError)
     end
 
     it "should fail when the specified terminus class cannot be found" do
         Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(nil)
-        proc { @indirection.terminus(:foo) }.should raise_error(ArgumentError)
+        proc { @indirection.terminus_class = :foo }.should raise_error(ArgumentError)
+    end
+
+    it "should select the specified terminus class if a terminus class name is provided" do
+        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus(:foo).should equal(@terminus)
+    end
+
+    it "should use the configured terminus class if no terminus name is specified" do
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus_class = :foo
+        @indirection.terminus().should equal(@terminus)
     end
 
     after do
@@ -163,30 +162,48 @@ describe Puppet::Indirector::Indirection, " when deciding whether to cache" do
     before do
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
         @terminus = mock 'terminus'
-        @indirection.stubs(:terminus).returns(@terminus)
+        @terminus_class = mock 'terminus class'
+        @terminus_class.stubs(:new).returns(@terminus)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:foo, :test).returns(@terminus_class)
+        @indirection.terminus_class = :foo
     end
 
-    it "should not use a cache if there no cache setting" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(false)
-        @indirection.expects(:cache).never
-        @terminus.stubs(:save)
-        @indirection.save(:whev)
+    it "should provide a method for setting the cache terminus class" do
+        @indirection.should respond_to(:cache_class=)
     end
 
-    it "should not use a cache if the cache setting is set to 'none'" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(true)
-        Puppet.settings.expects(:value).with("test_cache").returns("none")
+    it "should fail to cache if no cache type has been specified" do
+        proc { @indirection.cache }.should raise_error(Puppet::DevError)
+    end
+
+    it "should fail to set the cache class when the cache class name is an empty string" do
+        proc { @indirection.cache_class = "" }.should raise_error(ArgumentError)
+    end
+
+    it "should fail to set the cache class when the cache class name is nil" do
+        proc { @indirection.cache_class = nil }.should raise_error(ArgumentError)
+    end
+
+    it "should fail to set the cache class when the specified cache class cannot be found" do
+        Puppet::Indirector::Terminus.expects(:terminus_class).with(:foo, :test).returns(nil)
+        proc { @indirection.cache_class = :foo }.should raise_error(ArgumentError)
+    end
+
+    it "should not use a cache if there no cache setting" do
         @indirection.expects(:cache).never
         @terminus.stubs(:save)
         @indirection.save(:whev)
     end
 
-    it "should use a cache if there is a related cache setting and it is not set to 'none'" do
-        Puppet.settings.expects(:valid?).with("test_cache").returns(true)
-        Puppet.settings.expects(:value).with("test_cache").returns("something")
+    it "should use a cache if a cache was configured" do
         cache = mock 'cache'
         cache.expects(:save).with(:whev)
-        @indirection.expects(:cache).returns(cache)
+
+        cache_class = mock 'cache class'
+        cache_class.expects(:new).returns(cache)
+        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:mycache, :test).returns(cache_class)
+
+        @indirection.cache_class = :mycache
         @terminus.stubs(:save)
         @indirection.save(:whev)
     end
@@ -199,7 +216,6 @@ end
 
 describe Puppet::Indirector::Indirection, " when using a cache" do
     before do
-        Puppet.settings.stubs(:valid?).returns(true)
         Puppet.settings.stubs(:value).with("test_terminus").returns("test_terminus")
         @terminus_class = mock 'terminus_class'
         @terminus = mock 'terminus'
@@ -209,11 +225,12 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
         Puppet::Indirector::Terminus.stubs(:terminus_class).with(:cache_terminus, :test).returns(@cache_class)
         Puppet::Indirector::Terminus.stubs(:terminus_class).with(:test_terminus, :test).returns(@terminus_class)
         @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
+        @indirection.terminus_class = :test_terminus
     end
 
     it "should copy all writing indirection calls to the cache terminus" do
         @cache_class.expects(:new).returns(@cache)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
+        @indirection.cache_class = :cache_terminus
         @cache.expects(:save).with(:whev)
         @terminus.stubs(:save)
         @indirection.save(:whev)
@@ -227,6 +244,7 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
     it "should reuse the cache terminus" do
         @cache_class.expects(:new).returns(@cache)
         Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
+        @indirection.cache_class = :cache_terminus
         @indirection.cache.should equal(@cache)
         @indirection.cache.should equal(@cache)
     end
@@ -234,19 +252,7 @@ describe Puppet::Indirector::Indirection, " when using a cache" do
     it "should remove the cache terminus when all other terminus instances are cleared" do
         cache2 = mock 'cache2'
         @cache_class.stubs(:new).returns(@cache, cache2)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus")
-        @indirection.cache.should equal(@cache)
-        @indirection.clear_cache
-        @indirection.cache.should equal(cache2)
-    end
-
-    it "should look up the cache name when recreating the cache terminus after terminus instances have been cleared" do
-        cache_class2 = mock 'cache_class2'
-        cache2 = mock 'cache2'
-        cache_class2.expects(:new).returns(cache2)
-        @cache_class.expects(:new).returns(@cache)
-        Puppet::Indirector::Terminus.stubs(:terminus_class).with(:other_cache, :test).returns(cache_class2)
-        Puppet.settings.stubs(:value).with("test_cache").returns("cache_terminus", "other_cache")
+        @indirection.cache_class = :cache_terminus
         @indirection.cache.should equal(@cache)
         @indirection.clear_cache
         @indirection.cache.should equal(cache2)