Merge branch 'indirection' of http://git.rickbradley.com/puppet into indirection

2 files changed, 211 insertions, 132 deletions

diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index 13cf3991f..ca1b4b673 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -7,14 +7,18 @@ module Puppet::Indirector
     # LAK:FIXME We need to figure out how to handle documentation for the
     # different indirection types.
 
+# JRB:TODO factor this out into its own class, with specs, and require it here
+# require 'puppet/indirector/terminus'
+
     # A simple class that can function as the base class for indirected types.
     class Terminus
         require 'puppet/util/docs'
         extend Puppet::Util::Docs
-
+        
         class << self
             attr_accessor :name, :indirection
         end
+        
         def name
             self.class.name
         end
@@ -35,14 +39,18 @@ module Puppet::Indirector
     require 'puppet/util/instance_loader'
     extend Puppet::Util::InstanceLoader
 
+# JRB:TODO - where did this come from, re: the specs?  also, shouldn't this be protected/private?
+    
     # Register a given indirection type.  The classes including this module
     # handle creating terminus instances, but the module itself handles
     # loading them and managing the classes.
-    def self.register_indirection(name)
+    def self.enable_autoloading_indirection(indirection)
         # Set up autoloading of the appropriate termini.
-        instance_load name, "puppet/indirector/%s" % name
+        instance_load indirection, "puppet/indirector/%s" % indirection
     end
     
+# JRB:TODO -- where did this come from, re: the specs? also, any way to make this protected/private?
+    
     # Define a new indirection terminus.  This method is used by the individual
     # termini in their separate files.  Again, the autoloader takes care of
     # actually loading these files.
@@ -55,56 +63,101 @@ module Puppet::Indirector
             :hash => instance_hash(indirection),
             :attributes => options,
             :block => block,
-            :parent => options[:parent] || Terminus
+            :parent => options[:parent] || Terminus,
+# JRB:FIXME -- why do I have to use overwrite here?            
+            :overwrite => 'please do motherfucker'
         )
         klass.indirection = indirection
         klass.name = terminus
     end
 
+# JRB:TODO where did this come from, re: the specs?  also, shouldn't this be protected/private?    
     # Retrieve a terminus class by indirection and name.
+# JRB:FIXME -- should be protected/private
     def self.terminus(indirection, terminus)
         loaded_instance(indirection, terminus)
     end
+    
+    # clear out the list of known indirections
+#JRB:TODO -- I would prefer to get rid of this altogether, but it's implicated in testing, given the class loader
+    def self.reset
+      @indirections = {}
+      @class_indirections = {}
+    end
+    
+    # return a hash of registered indirections, keys are indirection names, values are classes which handle the indirections
+    def self.indirections
+      @indirections ||= {}
+      @indirections
+    end
+    
+    # associate an indirection name with the class which handles the indirection
+    def self.register_indirection(name, klass)
+      @indirections ||= {}
+      @class_indirections ||= {}
+      
+      raise ArgumentError, "Already performing an indirection of %s; cannot redirect %s" % [name, klass.name] if @indirections[name]
+      raise ArgumentError, "Class %s is already redirecting to %s; cannot redirect to %s" % 
+        [klass.name, @class_indirections[klass.name], name] if @class_indirections[klass.name]
+      @class_indirections[klass.name] = name
+      @indirections[name] = klass
+    end
+    
+    def self.terminus_for_indirection(name)
+# JRB:TODO make this do something useful, aka look something up in a .yml file
+      # JRB:TODO look up name + '_source' in standard configuration
+      :ldap
+    end
 
     # Declare that the including class indirects its methods to
     # this terminus.  The terminus name must be the name of a Puppet
     # default, not the value -- if it's the value, then it gets
     # evaluated at parse time, which is before the user has had a chance
     # to override it.
-    #   Options are:
-    # +:to+: What parameter to use as the name of the indirection terminus.
     def indirects(indirection, options = {})
-        if defined?(@indirection)
-            raise ArgumentError, "Already performing an indirection of %s; cannot redirect %s" % [@indirection.name, indirection]
-        end
+#JRB:TODO remove options hash  ^^^
 
-        # JRB:  this associates an indirection class with this class (e.g., Node.@indirection = Indirection.new(:node))
-        @indirection = Indirection.new(indirection, options)
-
-        # Set up autoloading of the appropriate termini.
-        Puppet::Indirector.register_indirection indirection
+        # associate the name :node, with this class, Node
+        # also, do error checking (already registered, etc.)
+        Puppet::Indirector.register_indirection(indirection, self)
 
+        # populate this registered class with the various new methods
         extend ClassMethods
         include InstanceMethods
-    end
 
-    module InstanceMethods
-      # these become instance methods 
-      def save
-      end
+        # look up the type of Terminus for this name (:node => :ldap)
+        terminus = Puppet::Indirector.terminus_for_indirection(indirection)
+
+        # instantiate the actual Terminus for that type and this name (:ldap, w/ args :node)
+        # & hook the instantiated Terminus into this registered class (Node: @indirection = terminus)
+        Puppet::Indirector.enable_autoloading_indirection indirection
+        @indirection = Puppet::Indirector.terminus(indirection, terminus)
     end
-    
-    module ClassMethods
-      def find
+
+    module ClassMethods   
+      attr_reader :indirection
+         
+      def find(*args)
+        self.indirection.find(args)
+        # JRB:TODO look up the indirection, and call its .find method
       end
 
-      def destroy
+      def destroy(*args)
+        self.indirection.destroy(args)
       end
 
-      def search
+      def search(*args)
+        self.indirection.search(args)
       end
     end
 
+    module InstanceMethods
+      # these become instance methods 
+      def save(*args)
+        self.class.indirection.save(args)
+      end
+    end
+    
     # JRB:FIXME: these methods to be deprecated:
 
     # Define methods for each of the HTTP methods.  These just point to the
@@ -117,23 +170,24 @@ module Puppet::Indirector
     # the method in question.  We should probably require that indirections
     # declare supported methods, and then verify that termini implement all of
     # those methods.
-    [:get, :post, :put, :delete].each do |method_name|
-        define_method(method_name) do |*args|
-            redirect(method_name, *args)
-        end
-    end
-
-    private
-
-
-    # Redirect one of our methods to the corresponding method on the Terminus
-    def redirect(method_name, *args)
-        begin
-            @indirection.terminus.send(method_name, *args)
-        rescue NoMethodError => detail
-            puts detail.backtrace if Puppet[:trace]
-            raise ArgumentError, "The %s terminus of the %s indirection failed to respond to %s: %s" %
-                [@indirection.terminus.name, @indirection.name, method_name, detail]
-        end
-    end
+    # [:get, :post, :put, :delete].each do |method_name|
+    #     define_method(method_name) do |*args|
+    #         redirect(method_name, *args)
+    #     end
+    # end
+    # 
+    # private
+    # 
+    # 
+    # # JRB:TODO this needs to be renamed, as it actually ends up on the model class, where it might conflict with something
+    # # Redirect one of our methods to the corresponding method on the Terminus
+    # def redirect(method_name, *args)
+    #     begin
+    #         @indirection.terminus.send(method_name, *args)
+    #     rescue NoMethodError => detail
+    #         puts detail.backtrace if Puppet[:trace]
+    #         raise ArgumentError, "The %s terminus of the %s indirection failed to respond to %s: %s" %
+    #             [@indirection.terminus.name, @indirection.name, method_name, detail]
+    #     end
+    # end
 end
diff --git a/spec/unit/indirector/indirector.rb b/spec/unit/indirector/indirector.rb
index 489ac57c4..78f1c7f73 100755
--- a/spec/unit/indirector/indirector.rb
+++ b/spec/unit/indirector/indirector.rb
@@ -1,138 +1,163 @@
-#!/usr/bin/env ruby
-
 require File.dirname(__FILE__) + '/../../spec_helper'
-
 require 'puppet/defaults'
 require 'puppet/indirector'
 
-class TestThingie
-  extend Puppet::Indirector
-  indirects :thingie
+describe Puppet::Indirector do
+  it "should provide a way to clear all registrations" do
+    Puppet::Indirector.should respond_to(:reset)
+  end
+  
+  it "should provide a way to access a list of all registered models" do
+    Puppet::Indirector.should respond_to(:indirections)
+  end
 end
 
-class TestNormalThingie
+describe Puppet::Indirector, "when no models are registered" do
+  before do
+    Puppet::Indirector.reset
+  end
+  
+  it "should provide an empty list of registered models" do
+    Puppet::Indirector.indirections.should == {}
+  end  
 end
 
-describe Puppet::Indirector, " when included into a class" do
+describe Puppet::Indirector, " when available to a model" do
   before do
-    @thingie = Class.new
-    @thingie.send(:extend, Puppet::Indirector)
+    @thingie = Class.new do
+      extend Puppet::Indirector
+    end
   end
 
-  it "should provide the indirects method to the class" do
+  it "should provide a way for the model to register an indirection under a name" do
     @thingie.should respond_to(:indirects)
   end
-  
-  it "should require a name to register when indirecting" do
+end
+
+describe Puppet::Indirector, "when registering an indirection" do
+  before do
+    Puppet::Indirector.reset
+    @thingie = Class.new do
+      extend Puppet::Indirector
+    end
+    Puppet::Indirector.stubs(:terminus_for_indirection).returns(:ldap)
+  end
+
+  it "should require a name when registering a model" do
     Proc.new {@thingie.send(:indirects) }.should raise_error(ArgumentError)
   end
   
-  it "should require each indirection to be registered under a unique name" do
+  it "should require each model to be registered under a unique name" do
     @thingie.send(:indirects, :name)
     Proc.new {@thingie.send(:indirects, :name)}.should raise_error(ArgumentError)
   end
   
-  it "should not allow a class to register multiple indirections" do
+  it "should not allow a model to register under multiple names" do
     @thingie.send(:indirects, :first)
     Proc.new {@thingie.send(:indirects, :second)}.should raise_error(ArgumentError)
   end
   
-  it "should provide a way to access the list of registered classes"
-  
-  it "should provide a way to find a class, given the registered name"
-  
-  it "should make a find method available on the registered class" do
+  it "should allow finding an instance of a model in a collection" do
     @thingie.send(:indirects, :first)
     @thingie.should respond_to(:find)
   end
     
-  it "should make a destroy method available on the registered class" do
+  it "should allow removing an instance of a model from a collection" do
     @thingie.send(:indirects, :first)
     @thingie.should respond_to(:destroy)
   end
   
-  it "should make a search method available on the registered class" do
+  it "should allow finding all matching model instances in a collection" do
     @thingie.send(:indirects, :first)
     @thingie.should respond_to(:search)
   end
   
-  it "should make a save method available on instances of the registered class" do
-    @thing = TestThingie.new
+  it "should allow for storing a model instance in a collection" do
+    @thing = Class.new do
+      extend Puppet::Indirector
+      indirects :thing
+    end.new
     @thing.should respond_to(:save)
   end
     
-
-  
-  # when dealing with Terminus methods
-  it "should look up the indirection configuration for the registered class when a new instance of that class is created"
+  it "should provide a way to get a handle to the terminus for a model" do
+    mock_terminus = mock('Terminus')
+    Puppet::Indirector.expects(:terminus_for_indirection).with(:node).returns(:ldap)
+    Puppet::Indirector.expects(:terminus).returns(mock_terminus)
+    @thingie.send(:indirects, :node)
+    @thingie.indirection.should == mock_terminus
+  end
   
-  it "should use the Terminus described in the class configuration"
+  it "should list the model in a list of known indirections" do
+    @thingie.send(:indirects, :name)
+    Puppet::Indirector.indirections[:name].should == @thingie
+  end  
   
-  it "should use the Terminus find method when calling find on the registered class"
-  it "should use the Terminus save method when calling save on the registered class"
-  it "should use the Terminus destroy method when calling destroy on the registered class"
-  it "should use the Terminus search method when calling search on the registered class"
-
-  it "should allow a registered class to specify its own means of ..."
-end
-
-
-
-
-
-
-
-describe Puppet::Indirector, " when managing indirections" do
-    before do
-        @indirector = Class.new
-        @indirector.send(:extend, Puppet::Indirector)
-    end
-
-    it "should create an indirection" do
-        indirection = @indirector.indirects :test, :to => :node_source
-        indirection.name.should == :test
-        indirection.to.should == :node_source
-    end
+  # when dealing with Terminus methods
+  it "should consult a per-model configuration to determine what kind of collection a model is being stored in" do
+    Puppet::Indirector.expects(:terminus_for_indirection).with(:node).returns(:ldap)
+    @thingie.send(:indirects, :node)
+  end
 
-    it "should not allow more than one indirection in the same object" do
-        @indirector.indirects :test
-        proc { @indirector.indirects :else }.should raise_error(ArgumentError)
-    end
+  it "should use the collection type described in the per-model configuration" do
+    mock_terminus = mock('Terminus')
+    Puppet::Indirector.expects(:terminus_for_indirection).with(:foo).returns(:bar)
+    Puppet::Indirector.expects(:terminus).with(:foo, :bar).returns(mock_terminus)
+    @thingie.send(:indirects, :foo)
+  end
+  
+  it "should handle lookups of a model instance by letting the terminus perform the lookup" do
+    @thingie.send(:indirects, :node)
+    mock_terminus = mock('Terminus')
+    mock_terminus.expects(:find)
+    @thingie.expects(:indirection).returns(mock_terminus)
+    @thingie.find
+  end
 
-    it "should allow multiple classes to use the same indirection" do
-        @indirector.indirects :test
-        other = Class.new
-        other.send(:extend, Puppet::Indirector)
-        proc { other.indirects :test }.should_not raise_error
-    end
+  it "should handle removing model instances from a collection letting the terminus remove the instance" do
+    @thingie.send(:indirects, :node)
+    mock_terminus = mock('Terminus')
+    mock_terminus.expects(:destroy)
+    @thingie.expects(:indirection).returns(mock_terminus)
+    @thingie.destroy  
+  end
+  
+  it "should handle searching for model instances by letting the terminus find the matching instances" do
+    @thingie.send(:indirects, :node)
+    mock_terminus = mock('Terminus')
+    mock_terminus.expects(:search)
+    @thingie.expects(:indirection).returns(mock_terminus)
+    @thingie.search    
+  end
+  
+  it "should handle storing a model instance by letting the terminus store the instance" do
+    @thingie.send(:indirects, :node)
+    mock_terminus = mock('Terminus')
+    mock_terminus.expects(:save)
+    @thingie.expects(:indirection).returns(mock_terminus)
+    @thingie.new.save        
+  end
+  
+  it "should provide the same terminus for a given registered model"
 
-    it "should should autoload termini from disk" do
-        Puppet::Indirector.expects(:instance_load).with(:test, "puppet/indirector/test")
-        @indirector.indirects :test
-    end
+  it "should not access the collection for a registered model until that collection is actually needed"
 
-    after do
-        Puppet.config.clear
-    end
+# TODO:  node lookup retries/searching
 end
 
-describe Puppet::Indirector, " when performing indirections" do
-    before do
-        @indirector = Class.new
-        @indirector.send(:extend, Puppet::Indirector)
-        @indirector.indirects :test, :to => :node_source
 
-        # Set up a fake terminus class that will just be used to spit out
-        # mock terminus objects.
-        @terminus_class = mock 'terminus_class'
-        Puppet::Indirector.stubs(:terminus).with(:test, :test_source).returns(@terminus_class)
-        Puppet[:node_source] = "test_source"
-    end
 
-    it "should redirect http methods to the default terminus" do
-        terminus = mock 'terminus'
-        terminus.expects(:put).with("myargument")
-        @terminus_class.expects(:new).returns(terminus)
-        @indirector.put("myargument")
-    end
-end
+
+# describe Puppet::Indirector::Terminus do
+#   it "should register itself"  # ???
+#   
+#   it "should allow for finding an object from a collection"
+#   it "should allow for finding matching objects from a collection"
+#   it "should allow for destroying an object in a collection"
+#   it "should allow an object to be saved to a collection"
+#   it "should allow an object class to pre-process its arguments"
+#   it "should allow an object class to be in a read-only collection"
+#   
+#   it "should look up the appropriate decorator for the class"
+#   it "should call "
+# end