diff options
| -rw-r--r-- | lib/puppet/indirector.rb | 13 | ||||
| -rw-r--r-- | lib/puppet/indirector/indirection.rb | 55 | ||||
| -rwxr-xr-x | spec/integration/node/facts.rb | 4 | ||||
| -rwxr-xr-x | spec/unit/indirector.rb | 33 | ||||
| -rwxr-xr-x | spec/unit/indirector/indirection.rb | 370 |
5 files changed, 275 insertions, 200 deletions
diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index a8a7a84d1..2402b9cbe 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -40,22 +40,27 @@ module Puppet::Indirector indirection.terminus_class = klass end + # Expire any cached instance. + def expire(*args) + indirection.expire *args + end + def find(*args) - indirection.find Puppet::Indirector::Request.new(indirection.name, :find, *args) + indirection.find *args end def destroy(*args) - indirection.destroy Puppet::Indirector::Request.new(indirection.name, :destroy, *args) + indirection.destroy *args end def search(*args) - indirection.search Puppet::Indirector::Request.new(indirection.name, :search, *args) + indirection.search *args end end module InstanceMethods def save(*args) - self.class.indirection.save Puppet::Indirector::Request.new(self.class.indirection.name, :save, self, *args) + self.class.indirection.save self, *args end end end diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 536735c21..ccb7ef6e7 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -127,8 +127,8 @@ class Puppet::Indirector::Indirection end # Set up our request object. - def request(key, method, arguments = nil) - Puppet::Indirector::Request.new(self.name, key, method, arguments) + def request(method, key, arguments = nil) + Puppet::Indirector::Request.new(self.name, method, key, arguments) end # Return the singleton terminus for this indirection. @@ -172,7 +172,26 @@ class Puppet::Indirector::Indirection end end - def find(request) + # Expire a cached object, if one is cached. Note that we don't actually + # remove it, we expire it and write it back out to disk. This way people + # can still use the expired object if they want. + def expire(key, *args) + request = request(:expire, key, *args) + + return nil unless cache? + + return nil unless instance = cache.find(key, *args) + + # Set an expiration date in the past + instance.expiration = Time.now - 60 + + cache.save(instance, *args) + end + + # Search for an instance in the appropriate terminus, caching the + # results if caching is configured.. + def find(key, *args) + request = request(:find, key, *args) terminus = prepare(request) # See if our instance is in the cache and up to date. @@ -190,9 +209,7 @@ class Puppet::Indirector::Indirection result.expiration ||= self.expiration if cache? Puppet.info "Caching %s for %s" % [self.name, request.key] - cached_request = request.clone - cached_request.instance = result - cache.save(cached_request) + cache.save request(:save, result, *args) end return result @@ -202,12 +219,14 @@ class Puppet::Indirector::Indirection end # Remove something via the terminus. - def destroy(request) + def destroy(key, *args) + request = request(:destroy, key, *args) terminus = prepare(request) terminus.destroy(request) - if cache? and cached = cache.find(request) + if cache? and cached = cache.find(request(:find, key, *args)) + # Reuse the existing request, since it's equivalent. cache.destroy(request) end @@ -215,17 +234,24 @@ class Puppet::Indirector::Indirection end # Search for more than one instance. Should always return an array. - def search(request) + def search(key, *args) + request = request(:search, key, *args) terminus = prepare(request) - result = terminus.search(request) + if result = terminus.search(request) + raise Puppet::DevError, "Search results from terminus %s are not an array" % terminus.name unless result.is_a?(Array) - result + result.each do |instance| + instance.expiration ||= self.expiration + end + return result + end end # Save the instance in the appropriate terminus. This method is # normally an instance method on the indirected class. - def save(request) + def save(instance, *args) + request = request(:save, instance, *args) terminus = prepare(request) # If caching is enabled, save our document there @@ -238,12 +264,15 @@ class Puppet::Indirector::Indirection # Check authorization if there's a hook available; fail if there is one # and it returns false. def check_authorization(request, terminus) + # At this point, we're assuming authorization makes no sense without + # client information. return unless request.options[:node] + # This is only to authorize via a terminus-specific authorization hook. return unless terminus.respond_to?(:authorized?) unless terminus.authorized?(request) - raise ArgumentError, "Not authorized to call %s with %s" % [request.method, request.options.inspect] + raise ArgumentError, "Not authorized to call %s on %s with %s" % [request.method, request.key, request.options.inspect] end end diff --git a/spec/integration/node/facts.rb b/spec/integration/node/facts.rb index 977a1b6c9..d065918be 100755 --- a/spec/integration/node/facts.rb +++ b/spec/integration/node/facts.rb @@ -9,6 +9,10 @@ describe Puppet::Node::Facts do describe "when using the indirector" do after { Puppet::Node::Facts.indirection.clear_cache } + it "should expire any cached node instances when it is saved" do + raise "This test fails" + end + it "should be able to delegate to the :yaml terminus" do Puppet::Node::Facts.indirection.stubs(:terminus_class).returns :yaml diff --git a/spec/unit/indirector.rb b/spec/unit/indirector.rb index 892e70c1f..1efa7b2e5 100755 --- a/spec/unit/indirector.rb +++ b/spec/unit/indirector.rb @@ -60,22 +60,14 @@ describe Puppet::Indirector, "when registering an indirection" do end describe "Delegated Indirection Method", :shared => true do - it "should create an indirection request with the indirection name, the method being delegated, and all of the arguments to the method call" do - Puppet::Indirector::Request.expects(:new).with(@indirection.name, @method, "me", :one => :two) - @indirection.stubs(@method) - @thingie.send(@method, "me", :one => :two) - end - it "should delegate to the indirection" do @indirection.expects(@method) @thingie.send(@method, "me") end - it "should pass the indirection's request instance to the indirection's method" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).returns request - @indirection.expects(@method).with(request) - @thingie.send(@method, "me") + it "should pass all of the passed arguments directly to the indirection instance" do + @indirection.expects(@method).with("me", :one => :two) + @thingie.send(@method, "me", :one => :two) end it "should return the results of the delegation as its result" do @@ -116,28 +108,25 @@ describe Puppet::Indirector, "when redirecting a model" do it_should_behave_like "Delegated Indirection Method" end + describe "when expiring instances via the model" do + before { @method = :expire } + it_should_behave_like "Delegated Indirection Method" + end + # This is an instance method, so it behaves a bit differently. describe "when saving instances via the model" do before do @instance = @thingie.new("me") end - it "should pass the method name, the instance, plus all passed arguments to the indirection's request method" do - Puppet::Indirector::Request.expects(:new).with(@indirection.name, :save, @instance, :one => :two) - @indirection.stubs(:save) - @instance.save(:one => :two) - end - it "should delegate to the indirection" do @indirection.expects(:save) @instance.save end - it "should pass the indirection's request instance to the indirection's method" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).returns request - @indirection.expects(:save).with(request) - @instance.save + it "should pass the instance and all arguments to the indirection's :save method" do + @indirection.expects(:save).with(@instance, :one => :two) + @instance.save :one => :two end it "should return the results of the delegation as its result" do diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb index f33444bbf..06d9fcb6d 100755 --- a/spec/unit/indirector/indirection.rb +++ b/spec/unit/indirector/indirection.rb @@ -4,6 +4,88 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/indirector/indirection' +describe "Indirection Delegator", :shared => true do + it "should create a request object with the appropriate method name and all of the passed arguments" do + request = stub 'request', :options => {} + + @indirection.expects(:request).with(@method, "mystuff", :one => :two).returns request + + @terminus.stubs(@method) + + @indirection.send(@method, "mystuff", :one => :two) + end + + it "should let the :select_terminus method choose the terminus using the created request if the :select_terminus method is available" do + # Define the method, so our respond_to? hook matches. + class << @indirection + def select_terminus(request) + end + end + + request = stub 'request', :key => "me", :options => {} + + @indirection.stubs(:request).returns request + + @indirection.expects(:select_terminus).with(request).returns :test_terminus + + @indirection.stubs(:check_authorization) + @terminus.expects(@method) + + @indirection.send(@method, "me") + end + + it "should choose the terminus returned by the :terminus_class method if no :select_terminus method is available" do + @indirection.expects(:terminus_class).returns :test_terminus + + @terminus.expects(@method) + + @indirection.send(@method, "me") + end + + it "should let the appropriate terminus perform the lookup" do + @terminus.expects(@method).with { |r| r.is_a?(Puppet::Indirector::Request) } + @indirection.send(@method, "me") + end +end + +describe "Delegation Authorizer", :shared => true do + before do + # So the :respond_to? turns out correctly. + class << @terminus + def authorized? + end + end + end + + it "should not check authorization if a node name is not provided" do + @terminus.expects(:authorized?).never + @terminus.stubs(@method) + + # The quotes are necessary here, else it looks like a block. + @request.stubs(:options).returns({}) + @indirection.send(@method, "/my/key") + end + + it "should pass the request to the terminus's authorization method" do + @terminus.expects(:authorized?).with { |r| r.is_a?(Puppet::Indirector::Request) }.returns(true) + @terminus.stubs(@method) + + @indirection.send(@method, "/my/key", :node => "mynode") + end + + it "should fail if authorization returns false" do + @terminus.expects(:authorized?).returns(false) + @terminus.stubs(@method) + proc { @indirection.send(@method, "/my/key", :node => "mynode") }.should raise_error(ArgumentError) + end + + it "should continue if authorization returns true" do + @terminus.expects(:authorized?).returns(true) + @terminus.stubs(@method) + @indirection.send(@method, "/my/key", :node => "mynode") + end +end + describe Puppet::Indirector::Indirection do describe "when initializing" do # (LAK) I've no idea how to test this, really. @@ -56,7 +138,8 @@ describe Puppet::Indirector::Indirection do @instance = stub 'instance', :expiration => nil, :expiration= => nil, :name => "whatever" @name = :mything - @request = stub 'instance', :key => "/my/key", :instance => @instance, :options => {} + #@request = stub 'instance', :key => "/my/key", :instance => @instance, :options => {} + @request = mock 'instance' end it "should allow setting the ttl" do @@ -109,30 +192,14 @@ describe Puppet::Indirector::Indirection do end describe "and looking for a model instance" do - it "should let the :select_terminus method choose the terminus if the method is defined" do - # Define the method, so our respond_to? hook matches. - class << @indirection - def select_terminus(request) - end - end + before { @method = :find } - @indirection.expects(:select_terminus).with(@request).returns :test_terminus + it_should_behave_like "Indirection Delegator" + it_should_behave_like "Delegation Authorizer" - @indirection.stubs(:check_authorization) - @terminus.expects(:find) - - @indirection.find(@request) - - end - - it "should let the appropriate terminus perform the lookup" do - @terminus.expects(:find).with(@request).returns(@instance) - @indirection.find(@request).should == @instance - end - - it "should return nil if nothing is returned by the terminus" do - @terminus.expects(:find).with(@request).returns(nil) - @indirection.find(@request).should be_nil + it "should return the results of the delegation" do + @terminus.expects(:find).returns(@instance) + @indirection.find("me").should equal(@instance) end it "should set the expiration date on any instances without one set" do @@ -143,7 +210,7 @@ describe Puppet::Indirector::Indirection do @instance.expects(:expiration).returns(nil) @instance.expects(:expiration=).with(:yay) - @indirection.find(@request) + @indirection.find("/my/key") end it "should not override an already-set expiration date on returned instances" do @@ -154,7 +221,7 @@ describe Puppet::Indirector::Indirection do @instance.expects(:expiration).returns(:yay) @instance.expects(:expiration=).never - @indirection.find(@request) + @indirection.find("/my/key") end describe "when caching is enabled" do @@ -166,24 +233,32 @@ describe Puppet::Indirector::Indirection do end it "should first look in the cache for an instance" do - @terminus.expects(:find).never - @cache.expects(:find).with(@request).returns @instance + @terminus.stubs(:find).never + @cache.expects(:find).returns @instance - @indirection.find(@request) + @indirection.find("/my/key") + end + + it "should use a request to look in the cache for cached objects" do + @cache.expects(:find).with { |r| r.method == :find and r.key == "/my/key" }.returns @instance + + @cache.stubs(:save) + + @indirection.find("/my/key") end it "should return the cached object if it is not expired" do @instance.stubs(:expired?).returns false @cache.stubs(:find).returns @instance - @indirection.find(@request).should equal(@instance) + @indirection.find("/my/key").should equal(@instance) end it "should send a debug log if it is using the cached object" do Puppet.expects(:debug) @cache.stubs(:find).returns @instance - @indirection.find(@request) + @indirection.find("/my/key") end it "should not return the cached object if it is expired" do @@ -191,7 +266,7 @@ describe Puppet::Indirector::Indirection do @cache.stubs(:find).returns @instance @terminus.stubs(:find).returns nil - @indirection.find(@request).should be_nil + @indirection.find("/my/key").should be_nil end it "should send an info log if it is using the cached object" do @@ -200,67 +275,59 @@ describe Puppet::Indirector::Indirection do @cache.stubs(:find).returns @instance @terminus.stubs(:find).returns nil - @indirection.find(@request) + @indirection.find("/my/key") end it "should cache any objects not retrieved from the cache" do - @cache.expects(:find).with(@request).returns nil + @cache.expects(:find).returns nil - @terminus.expects(:find).with(@request).returns(@instance) + @terminus.expects(:find).returns(@instance) @cache.expects(:save) - @request.expects(:clone).returns(stub('newreq', :instance= => nil)) + @indirection.find("/my/key") + end + + it "should use a request to look in the cache for cached objects" do + @cache.expects(:find).with { |r| r.method == :find and r.key == "/my/key" }.returns nil + + @terminus.stubs(:find).returns(@instance) + @cache.stubs(:save) - @indirection.find(@request) + @indirection.find("/my/key") end - it "should cache a clone of the request with the instance set to the cached object" do - @cache.expects(:find).with(@request).returns nil + it "should cache the instance using a request with the instance set to the cached object" do + @cache.stubs(:find).returns nil - newreq = mock 'request' - @request.expects(:clone).returns newreq - newreq.expects(:instance=).with(@instance) + @terminus.stubs(:find).returns(@instance) - @terminus.expects(:find).with(@request).returns(@instance) - @cache.expects(:save).with(newreq) + @cache.expects(:save).with { |r| r.method == :save and r.instance == @instance } - @indirection.find(@request) + @indirection.find("/my/key") end it "should send an info log that the object is being cached" do @cache.stubs(:find).returns nil - @request.expects(:clone).returns(stub('newreq', :instance= => nil)) - @terminus.stubs(:find).returns(@instance) @cache.stubs(:save) Puppet.expects(:info) - @indirection.find(@request) + @indirection.find("/my/key") end end end describe "and storing a model instance" do - it "should let the :select_terminus method choose the terminus if the method is defined" do - # Define the method, so our respond_to? hook matches. - class << @indirection - def select_terminus(req) - end - end - - @indirection.expects(:select_terminus).with(@request).returns :test_terminus + before { @method = :save } - @indirection.stubs(:check_authorization) - @terminus.expects(:save) + it_should_behave_like "Indirection Delegator" + it_should_behave_like "Delegation Authorizer" - @indirection.save(@request) - end - - it "should let the appropriate terminus store the instance" do - @terminus.expects(:save).with(@request).returns(@instance) - @indirection.save(@request).should == @instance + it "should return nil" do + @terminus.stubs(:save) + @indirection.save(@instance).should be_nil end describe "when caching is enabled" do @@ -271,38 +338,27 @@ describe Puppet::Indirector::Indirection do @instance.stubs(:expired?).returns false end - it "should save the object to the cache" do - @cache.expects(:save).with(@request) + it "should use a request to save the object to the cache" do + request = stub 'request', :instance => @instance, :options => {} + + @indirection.expects(:request).returns request + + @cache.expects(:save).with(request) @terminus.stubs(:save) - @indirection.save(@request) + @indirection.save(@instance) end end end describe "and removing a model instance" do - it "should let the :select_terminus method choose the terminus if the method is defined" do - # Define the method, so our respond_to? hook matches. - class << @indirection - def select_terminus(request) - end - end - - @indirection.expects(:select_terminus).with(@request).returns :test_terminus + before { @method = :destroy } - @indirection.stubs(:check_authorization) - @terminus.expects(:destroy) - - @indirection.destroy(@request) - end - - it "should delegate the instance removal to the appropriate terminus" do - @terminus.expects(:destroy).with(@request) - @indirection.destroy(@request) - end + it_should_behave_like "Indirection Delegator" + it_should_behave_like "Delegation Authorizer" it "should return nil" do @terminus.stubs(:destroy) - @indirection.destroy(@request).should be_nil + @indirection.destroy("/my/key").should be_nil end describe "when caching is enabled" do @@ -313,112 +369,104 @@ describe Puppet::Indirector::Indirection do @instance.stubs(:expired?).returns false end - it "should destroy any found object in the cache" do + it "should use a request instance to search in and remove objects from the cache" do + destroy = stub 'destroy_request', :key => "/my/key", :options => {} + find = stub 'destroy_request', :key => "/my/key", :options => {} + + @indirection.expects(:request).with(:destroy, "/my/key").returns destroy + @indirection.expects(:request).with(:find, "/my/key").returns find + cached = mock 'cache' - @cache.expects(:find).with(@request).returns cached - @cache.expects(:destroy).with(@request) + + @cache.expects(:find).with(find).returns cached + @cache.expects(:destroy).with(destroy) + @terminus.stubs(:destroy) - @indirection.destroy(@request) + @indirection.destroy("/my/key") end end end describe "and searching for multiple model instances" do - it "should let the :select_terminus method choose the terminus if the method is defined" do - # Define the method, so our respond_to? hook matches. - class << @indirection - def select_terminus(req) - end - end + before { @method = :search } - @indirection.expects(:select_terminus).with(@request).returns :test_terminus + it_should_behave_like "Indirection Delegator" + it_should_behave_like "Delegation Authorizer" + + it "should set the expiration date on any instances without one set" do + @terminus.stubs(:search).returns([@instance]) - @indirection.stubs(:check_authorization) - @terminus.expects(:search) + @indirection.expects(:expiration).returns :yay + + @instance.expects(:expiration).returns(nil) + @instance.expects(:expiration=).with(:yay) - @indirection.search(@request) + @indirection.search("/my/key") end - it "should let the appropriate terminus find the matching instances" do - @terminus.expects(:search).with(@request).returns(@instance) - @indirection.search(@request).should == @instance + it "should not override an already-set expiration date on returned instances" do + @terminus.stubs(:search).returns([@instance]) + + @indirection.expects(:expiration).never + + @instance.expects(:expiration).returns(:yay) + @instance.expects(:expiration=).never + + @indirection.search("/my/key") + end + + it "should return the results of searching in the terminus" do + @terminus.expects(:search).returns([@instance]) + @indirection.search("/my/key").should == [@instance] end end - describe "and an authorization hook is present" do - before do - # So the :respond_to? turns out correctly. - class << @terminus - def authorized? - end + describe "and expiring a model instance" do + describe "when caching is not enabled" do + it "should do nothing" do + @cache_class.expects(:new).never + + @indirection.expire("/my/key") end - - @request = stub 'instance', :key => "/my/key", :instance => @instance, :options => {:node => "mynode"} end - it "should not check authorization if a node name is not provided" do - @terminus.expects(:authorized?).never - @terminus.stubs(:find) + describe "when caching is enabled" do + before do + @indirection.cache_class = :cache_terminus + @cache_class.expects(:new).returns(@cache) - # The quotes are necessary here, else it looks like a block. - @request.stubs(:options).returns({}) - @indirection.find(@request) - end + @instance.stubs(:expired?).returns false + end - it "should pass the request to the terminus's authorization method" do - @terminus.expects(:authorized?).with(@request).returns(true) - @terminus.stubs(:find) + it "should do nothing if no such instance is cached" do + @cache.expects(:find).returns nil - @indirection.find(@request) - end + @indirection.expire("/my/key") + end - it "should fail while finding instances if authorization returns false" do - @terminus.expects(:authorized?).returns(false) - @terminus.stubs(:find) - proc { @indirection.find(@request) }.should raise_error(ArgumentError) - end + it "should set the cached instance's expiration to a time in the past" do + cached = mock 'cached' - it "should continue finding instances if authorization returns true" do - @terminus.expects(:authorized?).returns(true) - @terminus.stubs(:find) - @indirection.find(@request) - end + @cache.expects(:find).returns cached + @cache.stubs(:save) - it "should fail while saving instances if authorization returns false" do - @terminus.expects(:authorized?).returns(false) - @terminus.stubs(:save) - proc { @indirection.save(@request) }.should raise_error(ArgumentError) - end + cached.expects(:expiration=).with { |t| t < Time.now } - it "should continue saving instances if authorization returns true" do - @terminus.expects(:authorized?).returns(true) - @terminus.stubs(:save) - @indirection.save(@request) - end + @indirection.expire("/my/key") + end - it "should fail while destroying instances if authorization returns false" do - @terminus.expects(:authorized?).returns(false) - @terminus.stubs(:destroy) - proc { @indirection.destroy(@request) }.should raise_error(ArgumentError) - end + it "should save the now expired instance back into the cache" do + cached = stub 'cached', :expiration= => nil - it "should continue destroying instances if authorization returns true" do - @terminus.expects(:authorized?).returns(true) - @terminus.stubs(:destroy) - @indirection.destroy(@request) - end + @cache.expects(:find).returns cached - it "should fail while searching for instances if authorization returns false" do - @terminus.expects(:authorized?).returns(false) - @terminus.stubs(:search) - proc { @indirection.search(@request) }.should raise_error(ArgumentError) - end + cached.expects(:expiration=).with { |t| t < Time.now } + + @cache.expects(:save).with(cached) - it "should continue searching for instances if authorization returns true" do - @terminus.expects(:authorized?).returns(true) - @terminus.stubs(:search) - @indirection.search(@request) + @indirection.expire("/my/key") + end end end |