diff options
author | Luke Kanies <luke@madstop.com> | 2008-04-08 18:21:18 -0500 |
---|---|---|
committer | Luke Kanies <luke@madstop.com> | 2008-04-08 18:21:18 -0500 |
commit | bf728d23caca4f58ae4ede1a2d477c9fc15e0bdc (patch) | |
tree | d79ae6e23667f0a1b201537914a025c9f677aa74 /spec/unit/indirector | |
parent | 644d6baae132a097170631f90521e878e31a5a0a (diff) | |
download | puppet-bf728d23caca4f58ae4ede1a2d477c9fc15e0bdc.tar.gz puppet-bf728d23caca4f58ae4ede1a2d477c9fc15e0bdc.tar.xz puppet-bf728d23caca4f58ae4ede1a2d477c9fc15e0bdc.zip |
Intermediate commit.
This commit adds a Request instance into the indirection,
pushing it all the way to the terminus instances. It's
a big commit because it requires modifying every terminus class.
There are still some thorny design issues. In particular, who
should be responsible for making the request object? I've tried
having both the indirection class and the Indirector module creating
it, and both have their issues.
Also, the Catalog class previously allowed passing Node instances
directly to the find method, which is now no longer possible because
the Request class would treat the node as the instance being found.
We need the request class to have two modes, one when it's passed an
instance and one when it's passed a key.
Diffstat (limited to 'spec/unit/indirector')
-rwxr-xr-x | spec/unit/indirector/catalog/compiler.rb | 4 | ||||
-rwxr-xr-x | spec/unit/indirector/checksum/file.rb | 17 | ||||
-rwxr-xr-x | spec/unit/indirector/direct_file_server.rb | 24 | ||||
-rwxr-xr-x | spec/unit/indirector/file.rb | 50 | ||||
-rwxr-xr-x | spec/unit/indirector/file_metadata/file.rb | 50 | ||||
-rwxr-xr-x | spec/unit/indirector/indirection.rb | 215 | ||||
-rwxr-xr-x | spec/unit/indirector/memory.rb | 30 | ||||
-rwxr-xr-x | spec/unit/indirector/node/memory.rb | 5 | ||||
-rwxr-xr-x | spec/unit/indirector/plain.rb | 6 | ||||
-rwxr-xr-x | spec/unit/indirector/request.rb | 18 | ||||
-rwxr-xr-x | spec/unit/indirector/yaml.rb | 34 |
11 files changed, 211 insertions, 242 deletions
diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb index a4a0acd58..5a26302d1 100755 --- a/spec/unit/indirector/catalog/compiler.rb +++ b/spec/unit/indirector/catalog/compiler.rb @@ -75,7 +75,9 @@ describe Puppet::Node::Catalog::Compiler, " when finding nodes" do it "should fail if it cannot find the node" do @node.stubs :merge Puppet::Node.expects(:find_by_any_name).with(@name).returns(nil) - proc { @compiler.find(@name) }.should raise_error(Puppet::Error) + request = stub 'request', :key => @name + @compiler.find(request) + proc { @compiler.find(request) }.should raise_error(Puppet::Error) end end diff --git a/spec/unit/indirector/checksum/file.rb b/spec/unit/indirector/checksum/file.rb index 64425904f..857d7b050 100755 --- a/spec/unit/indirector/checksum/file.rb +++ b/spec/unit/indirector/checksum/file.rb @@ -38,6 +38,8 @@ describe Puppet::Checksum::File do Puppet.stubs(:[]).with(:bucketdir).returns(@dir) @path = @store.path(@value) + + @request = stub 'request', :key => @value end @@ -76,7 +78,7 @@ describe Puppet::Checksum::File do # The smallest test that will use the calculated path it "should look for the calculated path" do File.expects(:exist?).with(@path).returns(false) - @store.find(@value) + @store.find(@request) end it "should return an instance of Puppet::Checksum created with the content if the file exists" do @@ -87,18 +89,18 @@ describe Puppet::Checksum::File do File.expects(:exist?).with(@path).returns(true) File.expects(:read).with(@path).returns(content) - @store.find(@value).should equal(sum) + @store.find(@request).should equal(sum) end it "should return nil if no file is found" do File.expects(:exist?).with(@path).returns(false) - @store.find(@value).should be_nil + @store.find(@request).should be_nil end it "should fail intelligently if a found file cannot be read" do File.expects(:exist?).with(@path).returns(true) File.expects(:read).with(@path).raises(RuntimeError) - proc { @store.find(@value) }.should raise_error(Puppet::Error) + proc { @store.find(@request) }.should raise_error(Puppet::Error) end end @@ -112,7 +114,7 @@ describe Puppet::Checksum::File do File.expects(:open).with(@path, "w") file = stub 'file', :name => @value - @store.save(file) + @store.save(@request) end it "should make any directories necessary for storage" do @@ -122,8 +124,7 @@ describe Puppet::Checksum::File do File.expects(:directory?).with(File.dirname(@path)).returns(true) File.expects(:open).with(@path, "w") - file = stub 'file', :name => @value - @store.save(file) + @store.save(@request) end end @@ -132,7 +133,7 @@ describe Puppet::Checksum::File do File.expects(:exist?).with(@path).returns(true) File.expects(:unlink).with(@path) - @store.destroy(@value) + @store.destroy(@request) end end end diff --git a/spec/unit/indirector/direct_file_server.rb b/spec/unit/indirector/direct_file_server.rb index a89b938e7..a8583716a 100755 --- a/spec/unit/indirector/direct_file_server.rb +++ b/spec/unit/indirector/direct_file_server.rb @@ -23,19 +23,21 @@ describe Puppet::Indirector::DirectFileServer do @server = @direct_file_class.new @uri = "file:///my/local" + + @request = stub 'request', :key => @uri, :options => {} end describe Puppet::Indirector::DirectFileServer, "when finding a single file" do it "should return nil if the file does not exist" do FileTest.expects(:exists?).with("/my/local").returns false - @server.find(@uri).should be_nil + @server.find(@request).should be_nil end it "should return a Content instance created with the full path to the file if the file exists" do FileTest.expects(:exists?).with("/my/local").returns true @model.expects(:new).returns(:mycontent) - @server.find(@uri).should == :mycontent + @server.find(@request).should == :mycontent end end @@ -49,18 +51,20 @@ describe Puppet::Indirector::DirectFileServer do it "should create the Content instance with the original key as the key" do @model.expects(:new).with { |key, options| key == @uri }.returns(@data) - @server.find(@uri) + @server.find(@request) end it "should pass the full path to the instance" do @model.expects(:new).with { |key, options| options[:path] == "/my/local" }.returns(@data) - @server.find(@uri) + @server.find(@request) end it "should pass the :links setting on to the created Content instance if the file exists and there is a value for :links" do @model.expects(:new).returns(@data) @data.expects(:links=).with(:manage) - @server.find(@uri, :links => :manage) + + @request.stubs(:options).returns(:links => :manage) + @server.find(@request) end end @@ -68,25 +72,27 @@ describe Puppet::Indirector::DirectFileServer do it "should return nil if the file does not exist" do FileTest.expects(:exists?).with("/my/local").returns false - @server.find(@uri).should be_nil + @server.find(@request).should be_nil end it "should pass the original key to :path2instances" do FileTest.expects(:exists?).with("/my/local").returns true @server.expects(:path2instances).with { |uri, path, options| uri == @uri } - @server.search(@uri) + @server.search(@request) end it "should use :path2instances from the terminus_helper to return instances if the file exists" do FileTest.expects(:exists?).with("/my/local").returns true @server.expects(:path2instances) - @server.search(@uri) + @server.search(@request) end it "should pass any options on to :path2instances" do FileTest.expects(:exists?).with("/my/local").returns true @server.expects(:path2instances).with { |uri, path, options| options == {:testing => :one, :other => :two}} - @server.search(@uri, :testing => :one, :other => :two) + + @request.stubs(:options).returns(:testing => :one, :other => :two) + @server.search(@request) end end end diff --git a/spec/unit/indirector/file.rb b/spec/unit/indirector/file.rb index fa10743ef..67ead4cdb 100755 --- a/spec/unit/indirector/file.rb +++ b/spec/unit/indirector/file.rb @@ -21,6 +21,8 @@ describe Puppet::Indirector::File do @path = "/my/file" @dir = "/my" + + @request = stub 'request', :key => @path end describe Puppet::Indirector::File, " when finding files" do @@ -37,7 +39,7 @@ describe Puppet::Indirector::File do File.expects(:exist?).with(@path).returns(true) File.expects(:read).with(@path).returns(content) - @searcher.find(@path) + @searcher.find(@request) end it "should create the model instance with the content as the only argument to initialization" do @@ -48,18 +50,18 @@ describe Puppet::Indirector::File do File.expects(:exist?).with(@path).returns(true) File.expects(:read).with(@path).returns(content) - @searcher.find(@path).should equal(file) + @searcher.find(@request).should equal(file) end it "should return nil if no file is found" do File.expects(:exist?).with(@path).returns(false) - @searcher.find(@path).should be_nil + @searcher.find(@request).should be_nil end it "should fail intelligently if a found file cannot be read" do File.expects(:exist?).with(@path).returns(true) File.expects(:read).with(@path).raises(RuntimeError) - proc { @searcher.find(@path) }.should raise_error(Puppet::Error) + proc { @searcher.find(@request) }.should raise_error(Puppet::Error) end it "should use the path() method to calculate the path if it exists" do @@ -68,42 +70,39 @@ describe Puppet::Indirector::File do end File.expects(:exist?).with(@path.upcase).returns(false) - @searcher.find(@path) + @searcher.find(@request) end end describe Puppet::Indirector::File, " when saving files" do + before do + @content = "my content" + @file = stub 'file', :content => @content, :path => @path, :name => @path + @request.stubs(:instance).returns @file + end it "should provide a method to save file contents at a specified path" do filehandle = mock 'file' - content = "my content" File.expects(:directory?).with(@dir).returns(true) File.expects(:open).with(@path, "w").yields(filehandle) - filehandle.expects(:print).with(content) - - file = stub 'file', :content => content, :path => @path, :name => @path + filehandle.expects(:print).with(@content) - @searcher.save(file) + @searcher.save(@request) end it "should fail intelligently if the file's parent directory does not exist" do File.expects(:directory?).with(@dir).returns(false) - file = stub 'file', :path => @path, :name => @path - - proc { @searcher.save(file) }.should raise_error(Puppet::Error) + proc { @searcher.save(@request) }.should raise_error(Puppet::Error) end it "should fail intelligently if a file cannot be written" do filehandle = mock 'file' - content = "my content" File.expects(:directory?).with(@dir).returns(true) File.expects(:open).with(@path, "w").yields(filehandle) - filehandle.expects(:print).with(content).raises(ArgumentError) - - file = stub 'file', :content => content, :path => @path, :name => @path + filehandle.expects(:print).with(@content).raises(ArgumentError) - proc { @searcher.save(file) }.should raise_error(Puppet::Error) + proc { @searcher.save(@request) }.should raise_error(Puppet::Error) end it "should use the path() method to calculate the path if it exists" do @@ -111,10 +110,11 @@ describe Puppet::Indirector::File do name.upcase end - file = stub 'file', :name => "/yay" + # Reset the key to something without a parent dir, so no checks are necessary + @request.stubs(:key).returns "/my" - File.expects(:open).with("/YAY", "w") - @searcher.save(file) + File.expects(:open).with("/MY", "w") + @searcher.save(@request) end end @@ -124,20 +124,20 @@ describe Puppet::Indirector::File do File.expects(:exist?).with(@path).returns(true) File.expects(:unlink).with(@path) - @searcher.destroy(@path) + @searcher.destroy(@request) end it "should throw an exception if the file is not found" do File.expects(:exist?).with(@path).returns(false) - proc { @searcher.destroy(@path) }.should raise_error(Puppet::Error) + proc { @searcher.destroy(@request) }.should raise_error(Puppet::Error) end it "should fail intelligently if the file cannot be removed" do File.expects(:exist?).with(@path).returns(true) File.expects(:unlink).with(@path).raises(ArgumentError) - proc { @searcher.destroy(@path) }.should raise_error(Puppet::Error) + proc { @searcher.destroy(@request) }.should raise_error(Puppet::Error) end it "should use the path() method to calculate the path if it exists" do @@ -148,7 +148,7 @@ describe Puppet::Indirector::File do File.expects(:exist?).with("/MY/FILE").returns(true) File.expects(:unlink).with("/MY/FILE") - @searcher.destroy(@path) + @searcher.destroy(@request) end end end diff --git a/spec/unit/indirector/file_metadata/file.rb b/spec/unit/indirector/file_metadata/file.rb index 0a37a6895..9474620c7 100755 --- a/spec/unit/indirector/file_metadata/file.rb +++ b/spec/unit/indirector/file_metadata/file.rb @@ -15,34 +15,38 @@ describe Puppet::Indirector::FileMetadata::File do it "should be a subclass of the DirectFileServer terminus" do Puppet::Indirector::FileMetadata::File.superclass.should equal(Puppet::Indirector::DirectFileServer) end -end -describe Puppet::Indirector::FileMetadata::File, "when creating the instance for a single found file" do - before do - @metadata = Puppet::Indirector::FileMetadata::File.new - @uri = "file:///my/local" - @data = mock 'metadata' - @data.stubs(:collect_attributes) - FileTest.expects(:exists?).with("/my/local").returns true - end + describe "when creating the instance for a single found file" do + before do + @metadata = Puppet::Indirector::FileMetadata::File.new + @uri = "file:///my/local" + @data = mock 'metadata' + @data.stubs(:collect_attributes) + FileTest.expects(:exists?).with("/my/local").returns true - it "should collect its attributes when a file is found" do - @data.expects(:collect_attributes) + @request = stub 'request', :key => @uri, :options => {} + end - Puppet::FileServing::Metadata.expects(:new).returns(@data) - @metadata.find(@uri).should == @data - end -end + it "should collect its attributes when a file is found" do + @data.expects(:collect_attributes) -describe Puppet::Indirector::FileMetadata::File, "when searching for multiple files" do - before do - @metadata = Puppet::Indirector::FileMetadata::File.new - @uri = "file:///my/local" + Puppet::FileServing::Metadata.expects(:new).returns(@data) + @metadata.find(@request).should == @data + end end - it "should collect the attributes of the instances returned" do - FileTest.expects(:exists?).with("/my/local").returns true - @metadata.expects(:path2instances).returns( [mock("one", :collect_attributes => nil), mock("two", :collect_attributes => nil)] ) - @metadata.search(@uri) + describe "when searching for multiple files" do + before do + @metadata = Puppet::Indirector::FileMetadata::File.new + @uri = "file:///my/local" + + @request = stub 'request', :key => @uri, :options => {} + end + + it "should collect the attributes of the instances returned" do + FileTest.expects(:exists?).with("/my/local").returns true + @metadata.expects(:path2instances).returns( [mock("one", :collect_attributes => nil), mock("two", :collect_attributes => nil)] ) + @metadata.search(@request) + end end end diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb index 8768076c6..f33444bbf 100755 --- a/spec/unit/indirector/indirection.rb +++ b/spec/unit/indirector/indirection.rb @@ -55,6 +55,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 => {} end it "should allow setting the ttl" do @@ -73,57 +75,67 @@ describe Puppet::Indirector::Indirection do Time.stubs(:now).returns now @indirection.expiration.should == (Time.now + 100) end - - describe "and looking for a model instance" do - it "should create a request with the indirection name, the sought-after name, the :find method, and any passed arguments" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).with(@indirection.name, @name, :find, {:one => :two}).returns request - @indirection.stubs(:check_authorization) - @terminus.stubs(:find) + it "should have a method for creating an indirection request instance" do + @indirection.should respond_to(:request) + end - @indirection.find(@name, :one => :two) + describe "creates a request" do + it "should create it with its name as the request's indirection name" do + Puppet::Indirector::Request.expects(:new).with { |name, *other| @indirection.name == name } + @indirection.request(:funtest, "yayness") end - it "should let the :select_terminus method choose the terminus if the method is defined" do + it "should require a method and key" do + Puppet::Indirector::Request.expects(:new).with { |name, method, key, *other| method == :funtest and key == "yayness" } + @indirection.request(:funtest, "yayness") + end + + it "should support optional arguments" do + Puppet::Indirector::Request.expects(:new).with { |name, method, key, other| other == {:one => :two} } + @indirection.request(:funtest, "yayness", :one => :two) + end + + it "should default to the arguments being nil" do + Puppet::Indirector::Request.expects(:new).with { |name, method, key, args| args.nil? } + @indirection.request(:funtest, "yayness") + end + + it "should return the request" do request = mock 'request' Puppet::Indirector::Request.expects(:new).returns request - + @indirection.request(:funtest, "yayness").should equal(request) + end + 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 - @indirection.expects(:select_terminus).with(request).returns :test_terminus + @indirection.expects(:select_terminus).with(@request).returns :test_terminus @indirection.stubs(:check_authorization) @terminus.expects(:find) - @indirection.find(@name) + @indirection.find(@request) end it "should let the appropriate terminus perform the lookup" do - @terminus.expects(:find).with(@name).returns(@instance) - @indirection.find(@name).should == @instance + @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(@name).returns(nil) - @indirection.find(@name).should be_nil - end - - it "should extend any found instance with the Envelope module" do - @terminus.stubs(:find).returns(@instance) - - @instance.expects(:extend).with(Puppet::Indirector::Envelope) - @indirection.find(@name) + @terminus.expects(:find).with(@request).returns(nil) + @indirection.find(@request).should be_nil end it "should set the expiration date on any instances without one set" do - # Otherwise, our stub method doesn't get used, so the tests fail. - @instance.stubs(:extend) @terminus.stubs(:find).returns(@instance) @indirection.expects(:expiration).returns :yay @@ -131,12 +143,10 @@ describe Puppet::Indirector::Indirection do @instance.expects(:expiration).returns(nil) @instance.expects(:expiration=).with(:yay) - @indirection.find(@name) + @indirection.find(@request) end it "should not override an already-set expiration date on returned instances" do - # Otherwise, our stub method doesn't get used, so the tests fail. - @instance.stubs(:extend) @terminus.stubs(:find).returns(@instance) @indirection.expects(:expiration).never @@ -144,7 +154,7 @@ describe Puppet::Indirector::Indirection do @instance.expects(:expiration).returns(:yay) @instance.expects(:expiration=).never - @indirection.find(@name) + @indirection.find(@request) end describe "when caching is enabled" do @@ -157,23 +167,23 @@ describe Puppet::Indirector::Indirection do it "should first look in the cache for an instance" do @terminus.expects(:find).never - @cache.expects(:find).with(@name).returns @instance + @cache.expects(:find).with(@request).returns @instance - @indirection.find(@name) + @indirection.find(@request) 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(@name).should equal(@instance) + @indirection.find(@request).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(@name) + @indirection.find(@request) end it "should not return the cached object if it is expired" do @@ -181,7 +191,7 @@ describe Puppet::Indirector::Indirection do @cache.stubs(:find).returns @instance @terminus.stubs(:find).returns nil - @indirection.find(@name).should be_nil + @indirection.find(@request).should be_nil end it "should send an info log if it is using the cached object" do @@ -190,63 +200,67 @@ describe Puppet::Indirector::Indirection do @cache.stubs(:find).returns @instance @terminus.stubs(:find).returns nil - @indirection.find(@name) + @indirection.find(@request) end it "should cache any objects not retrieved from the cache" do - @cache.expects(:find).with(@name).returns nil + @cache.expects(:find).with(@request).returns nil - @terminus.expects(:find).with(@name).returns(@instance) - @cache.expects(:save).with(@instance) + @terminus.expects(:find).with(@request).returns(@instance) + @cache.expects(:save) - @indirection.find(@name) + @request.expects(:clone).returns(stub('newreq', :instance= => nil)) + + @indirection.find(@request) + 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 + + newreq = mock 'request' + @request.expects(:clone).returns newreq + newreq.expects(:instance=).with(@instance) + + @terminus.expects(:find).with(@request).returns(@instance) + @cache.expects(:save).with(newreq) + + @indirection.find(@request) 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(@name) + @indirection.find(@request) end end end describe "and storing a model instance" do - it "should create a request with the indirection name, the instance's name, the :save method, and any passed arguments" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).with(@indirection.name, @instance.name, :save, {:one => :two}).returns request - - @indirection.stubs(:check_authorization) - @terminus.stubs(:save) - - @indirection.save(@instance, :one => :two) - end - it "should let the :select_terminus method choose the terminus if the method is defined" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).returns request - # Define the method, so our respond_to? hook matches. class << @indirection - def select_terminus(request) + def select_terminus(req) end end - @indirection.expects(:select_terminus).with(request).returns :test_terminus + @indirection.expects(:select_terminus).with(@request).returns :test_terminus @indirection.stubs(:check_authorization) @terminus.expects(:save) - @indirection.save(@instance) + @indirection.save(@request) end it "should let the appropriate terminus store the instance" do - @terminus.expects(:save).with(@instance).returns(@instance) - @indirection.save(@instance).should == @instance + @terminus.expects(:save).with(@request).returns(@instance) + @indirection.save(@request).should == @instance end describe "when caching is enabled" do @@ -258,50 +272,37 @@ describe Puppet::Indirector::Indirection do end it "should save the object to the cache" do - @cache.expects(:save).with(@instance) + @cache.expects(:save).with(@request) @terminus.stubs(:save) - @indirection.save(@instance) + @indirection.save(@request) end end end describe "and removing a model instance" do - it "should create a request with the indirection name, the name of the instance being destroyed, the :destroy method, and any passed arguments" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).with(@indirection.name, "me", :destroy, {:one => :two}).returns request - - @indirection.stubs(:check_authorization) - @terminus.stubs(:destroy) - - @indirection.destroy("me", :one => :two) - end - it "should let the :select_terminus method choose the terminus if the method is defined" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).returns request - # 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 + @indirection.expects(:select_terminus).with(@request).returns :test_terminus @indirection.stubs(:check_authorization) @terminus.expects(:destroy) - @indirection.destroy(@name) + @indirection.destroy(@request) end it "should delegate the instance removal to the appropriate terminus" do - @terminus.expects(:destroy).with(@name) - @indirection.destroy(@name) + @terminus.expects(:destroy).with(@request) + @indirection.destroy(@request) end it "should return nil" do @terminus.stubs(:destroy) - @indirection.destroy(@name).should be_nil + @indirection.destroy(@request).should be_nil end describe "when caching is enabled" do @@ -314,47 +315,34 @@ describe Puppet::Indirector::Indirection do it "should destroy any found object in the cache" do cached = mock 'cache' - @cache.expects(:find).with(@name).returns cached - @cache.expects(:destroy).with(@name) + @cache.expects(:find).with(@request).returns cached + @cache.expects(:destroy).with(@request) @terminus.stubs(:destroy) - @indirection.destroy(@name) + @indirection.destroy(@request) end end end describe "and searching for multiple model instances" do - it "should create a request with the indirection name, the search key, the :search method, and any passed arguments" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).with(@indirection.name, "me", :search, {:one => :two}).returns request - - @indirection.stubs(:check_authorization) - @terminus.stubs(:search) - - @indirection.search("me", :one => :two) - end - it "should let the :select_terminus method choose the terminus if the method is defined" do - request = mock 'request' - Puppet::Indirector::Request.expects(:new).returns request - # Define the method, so our respond_to? hook matches. class << @indirection - def select_terminus(request) + def select_terminus(req) end end - @indirection.expects(:select_terminus).with(request).returns :test_terminus + @indirection.expects(:select_terminus).with(@request).returns :test_terminus @indirection.stubs(:check_authorization) @terminus.expects(:search) - @indirection.search("me") + @indirection.search(@request) end it "should let the appropriate terminus find the matching instances" do - @terminus.expects(:search).with(@name).returns(@instance) - @indirection.search(@name).should == @instance + @terminus.expects(:search).with(@request).returns(@instance) + @indirection.search(@request).should == @instance end end @@ -365,69 +353,72 @@ describe Puppet::Indirector::Indirection do def authorized? end 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) - @indirection.find("/my/key") + + # The quotes are necessary here, else it looks like a block. + @request.stubs(:options).returns({}) + @indirection.find(@request) end it "should pass the request to the terminus's authorization method" do - request = stub 'request', :options => {:node => "yayhost"} - Puppet::Indirector::Request.expects(:new).returns(request) - @terminus.expects(:authorized?).with(request).returns(true) + @terminus.expects(:authorized?).with(@request).returns(true) @terminus.stubs(:find) - @indirection.find("/my/key", :node => "mynode") + @indirection.find(@request) end it "should fail while finding instances if authorization returns false" do @terminus.expects(:authorized?).returns(false) @terminus.stubs(:find) - proc { @indirection.find("/my/key", :node => "mynode") }.should raise_error(ArgumentError) + proc { @indirection.find(@request) }.should raise_error(ArgumentError) end it "should continue finding instances if authorization returns true" do @terminus.expects(:authorized?).returns(true) @terminus.stubs(:find) - @indirection.find("/my/key", :node => "mynode") + @indirection.find(@request) end it "should fail while saving instances if authorization returns false" do @terminus.expects(:authorized?).returns(false) @terminus.stubs(:save) - proc { @indirection.save(@instance, :node => "mynode") }.should raise_error(ArgumentError) + proc { @indirection.save(@request) }.should raise_error(ArgumentError) end it "should continue saving instances if authorization returns true" do @terminus.expects(:authorized?).returns(true) @terminus.stubs(:save) - @indirection.save(@instance, :node => "mynode") + @indirection.save(@request) end it "should fail while destroying instances if authorization returns false" do @terminus.expects(:authorized?).returns(false) @terminus.stubs(:destroy) - proc { @indirection.destroy("/my/key", :node => "mynode") }.should raise_error(ArgumentError) + proc { @indirection.destroy(@request) }.should raise_error(ArgumentError) end it "should continue destroying instances if authorization returns true" do @terminus.expects(:authorized?).returns(true) @terminus.stubs(:destroy) - @indirection.destroy(@instance, :node => "mynode") + @indirection.destroy(@request) end it "should fail while searching for instances if authorization returns false" do @terminus.expects(:authorized?).returns(false) @terminus.stubs(:search) - proc { @indirection.search("/my/key", :node => "mynode") }.should raise_error(ArgumentError) + proc { @indirection.search(@request) }.should raise_error(ArgumentError) end it "should continue searching for instances if authorization returns true" do @terminus.expects(:authorized?).returns(true) @terminus.stubs(:search) - @indirection.search("/my/key", :node => "mynode") + @indirection.search(@request) end end diff --git a/spec/unit/indirector/memory.rb b/spec/unit/indirector/memory.rb index 2e9a7f8f3..3b754a1eb 100755 --- a/spec/unit/indirector/memory.rb +++ b/spec/unit/indirector/memory.rb @@ -3,33 +3,7 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/indirector/memory' -describe "A Memory Terminus", :shared => true do - it "should find no instances by default" do - @searcher.find(@name).should be_nil - end - - it "should be able to find instances that were previously saved" do - @searcher.save(@instance) - @searcher.find(@name).should equal(@instance) - end - - it "should replace existing saved instances when a new instance with the same name is saved" do - @searcher.save(@instance) - two = stub 'second', :name => @name - @searcher.save(two) - @searcher.find(@name).should equal(two) - end - - it "should be able to remove previously saved instances" do - @searcher.save(@instance) - @searcher.destroy(@instance.name) - @searcher.find(@name).should be_nil - end - - it "should fail when asked to destroy an instance that does not exist" do - proc { @searcher.destroy(@instance) }.should raise_error(ArgumentError) - end -end +require 'shared_behaviours/memory_terminus' describe Puppet::Indirector::Memory do it_should_behave_like "A Memory Terminus" @@ -49,5 +23,7 @@ describe Puppet::Indirector::Memory do @searcher = @memory_class.new @name = "me" @instance = stub 'instance', :name => @name + + @request = stub 'request', :key => @name, :instance => @instance end end diff --git a/spec/unit/indirector/node/memory.rb b/spec/unit/indirector/node/memory.rb index a924c6209..71e01d4f3 100755 --- a/spec/unit/indirector/node/memory.rb +++ b/spec/unit/indirector/node/memory.rb @@ -4,14 +4,15 @@ require File.dirname(__FILE__) + '/../../../spec_helper' require 'puppet/indirector/node/memory' -# All of our behaviour is described here, so we always have to include it. -require File.dirname(__FILE__) + '/../memory' +require 'shared_behaviours/memory_terminus' describe Puppet::Node::Memory do before do @name = "me" @searcher = Puppet::Node::Memory.new @instance = stub 'instance', :name => @name + + @request = stub 'request', :key => @name, :instance => @instance end it_should_behave_like "A Memory Terminus" diff --git a/spec/unit/indirector/plain.rb b/spec/unit/indirector/plain.rb index 1277739af..aca2816f2 100755 --- a/spec/unit/indirector/plain.rb +++ b/spec/unit/indirector/plain.rb @@ -17,11 +17,13 @@ describe Puppet::Indirector::Plain do end @searcher = @plain_class.new + + @request = stub 'request', :key => "yay" end it "should return return an instance of the indirected model" do object = mock 'object' - @model.expects(:new).with("yay").returns object - @searcher.find("yay").should equal(object) + @model.expects(:new).with(@request.key).returns object + @searcher.find(@request).should equal(object) end end diff --git a/spec/unit/indirector/request.rb b/spec/unit/indirector/request.rb index fd57c5297..cdb40b181 100755 --- a/spec/unit/indirector/request.rb +++ b/spec/unit/indirector/request.rb @@ -10,45 +10,45 @@ describe Puppet::Indirector::Request do end it "should use provided value as the key if it is a string" do - Puppet::Indirector::Request.new(:ind, "mykey", :method).key.should == "mykey" + Puppet::Indirector::Request.new(:ind, :method, "mykey").key.should == "mykey" end it "should use provided value as the key if it is a symbol" do - Puppet::Indirector::Request.new(:ind, :mykey, :method).key.should == :mykey + Puppet::Indirector::Request.new(:ind, :method, :mykey).key.should == :mykey end it "should use the name of the provided instance as its key if an instance is provided as the key instead of a string" do instance = mock 'instance', :name => "mykey" - request = Puppet::Indirector::Request.new(:ind, instance, :method) + request = Puppet::Indirector::Request.new(:ind, :method, instance) request.key.should == "mykey" request.instance.should equal(instance) end it "should support options specified as a hash" do - lambda { Puppet::Indirector::Request.new(:ind, :key, :method, :one => :two) }.should_not raise_error(ArgumentError) + lambda { Puppet::Indirector::Request.new(:ind, :method, :key, :one => :two) }.should_not raise_error(ArgumentError) end it "should support nil options" do - lambda { Puppet::Indirector::Request.new(:ind, :key, :method, nil) }.should_not raise_error(ArgumentError) + lambda { Puppet::Indirector::Request.new(:ind, :method, :key, nil) }.should_not raise_error(ArgumentError) end it "should support unspecified options" do - lambda { Puppet::Indirector::Request.new(:ind, :key, :method) }.should_not raise_error(ArgumentError) + lambda { Puppet::Indirector::Request.new(:ind, :method, :key) }.should_not raise_error(ArgumentError) end it "should fail if options are specified as anything other than nil or a hash" do - lambda { Puppet::Indirector::Request.new(:ind, :key, :method, [:one, :two]) }.should raise_error(ArgumentError) + lambda { Puppet::Indirector::Request.new(:ind, :method, :key, [:one, :two]) }.should raise_error(ArgumentError) end it "should use an empty options hash if nil was provided" do - Puppet::Indirector::Request.new(:ind, :key, :method, nil).options.should == {} + Puppet::Indirector::Request.new(:ind, :method, :key, nil).options.should == {} end end it "should look use the Indirection class to return the appropriate indirection" do ind = mock 'indirection' Puppet::Indirector::Indirection.expects(:instance).with(:myind).returns ind - request = Puppet::Indirector::Request.new(:myind, :key, :method) + request = Puppet::Indirector::Request.new(:myind, :method, :key) request.indirection.should equal(ind) end diff --git a/spec/unit/indirector/yaml.rb b/spec/unit/indirector/yaml.rb index 339529ab0..53d12f426 100755 --- a/spec/unit/indirector/yaml.rb +++ b/spec/unit/indirector/yaml.rb @@ -21,37 +21,28 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do @dir = "/what/ever" Puppet.settings.stubs(:value).with(:yamldir).returns(@dir) - end - - it "should use the mtime of the written file as the version" do - stat = mock 'stat' - FileTest.stubs(:exist?).returns true - File.expects(:stat).returns stat - time = Time.now - stat.expects(:mtime).returns time - @store.version(:me).should equal(time) + @request = stub 'request', :key => :me, :instance => @subject end describe Puppet::Indirector::Yaml, " when choosing file location" do - it "should store all files in a single file root set in the Puppet defaults" do - @store.send(:path, :me).should =~ %r{^#{@dir}} + @store.path(:me).should =~ %r{^#{@dir}} end it "should use the terminus name for choosing the subdirectory" do - @store.send(:path, :me).should =~ %r{^#{@dir}/my_yaml} + @store.path(:me).should =~ %r{^#{@dir}/my_yaml} end it "should use the object's name to determine the file name" do - @store.send(:path, :me).should =~ %r{me.yaml$} + @store.path(:me).should =~ %r{me.yaml$} end end describe Puppet::Indirector::Yaml, " when storing objects as YAML" do - it "should only store objects that respond to :name" do - proc { @store.save(Object.new) }.should raise_error(ArgumentError) + @request.stubs(:instance).returns Object.new + proc { @store.save(@request) }.should raise_error(ArgumentError) end it "should convert Ruby objects to YAML and write them to disk" do @@ -62,7 +53,7 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do File.expects(:open).with(path, "w", 0660).yields(file) file.expects(:print).with(yaml) - @store.save(@subject) + @store.save(@request) end it "should create the indirection subdirectory if it does not exist" do @@ -75,16 +66,11 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do File.expects(:open).with(path, "w", 0660).yields(file) file.expects(:print).with(yaml) - @store.save(@subject) + @store.save(@request) end end describe Puppet::Indirector::Yaml, " when retrieving YAML" do - - it "should require the name of the object to retrieve" do - proc { @store.find(nil) }.should raise_error(ArgumentError) - end - it "should read YAML in from disk and convert it to Ruby objects" do path = @store.send(:path, @subject.name) @@ -92,7 +78,7 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do FileTest.expects(:exist?).with(path).returns(true) File.expects(:read).with(path).returns(yaml) - @store.find(@subject.name).instance_variable_get("@name").should == :me + @store.find(@request).instance_variable_get("@name").should == :me end it "should fail coherently when the stored YAML is invalid" do @@ -104,7 +90,7 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do FileTest.expects(:exist?).with(path).returns(true) File.expects(:read).with(path).returns(yaml) - proc { @store.find(@subject.name) }.should raise_error(Puppet::Error) + proc { @store.find(@request) }.should raise_error(Puppet::Error) end end end |