From 2cdd0f89a8d6687fafa77bf119cf2bbeed9d5b71 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 19 Feb 2008 12:49:57 -0600 Subject: puppet-compliant indentation --- lib/puppet/network/server.rb | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb index 50e3bd686..9e6b82469 100644 --- a/lib/puppet/network/server.rb +++ b/lib/puppet/network/server.rb @@ -8,44 +8,44 @@ class Puppet::Network::Server raise(ArgumentError, "Must specify :address or configure Puppet :bindaddress.") @port = args[:port] || Puppet[:masterport] || raise(ArgumentError, "Must specify :port or configure Puppet :masterport") - @protocols = [] - @listening = false - @routes = {} - self.register(args[:handlers]) if args[:handlers] + @protocols = [] + @listening = false + @routes = {} + self.register(args[:handlers]) if args[:handlers] end def register(*indirections) - raise ArgumentError, "Indirection names are required." if indirections.empty? - indirections.flatten.each { |i| @routes[i.to_sym] = true } + raise ArgumentError, "Indirection names are required." if indirections.empty? + indirections.flatten.each { |i| @routes[i.to_sym] = true } end def unregister(*indirections) - raise "Cannot unregister indirections while server is listening." if listening? - indirections = @routes.keys if indirections.empty? - - indirections.flatten.each do |i| - raise(ArgumentError, "Indirection [%s] is unknown." % i) unless @routes[i.to_sym] - end + raise "Cannot unregister indirections while server is listening." if listening? + indirections = @routes.keys if indirections.empty? + + indirections.flatten.each do |i| + raise(ArgumentError, "Indirection [%s] is unknown." % i) unless @routes[i.to_sym] + end - indirections.flatten.each do |i| - @routes.delete(i.to_sym) - end + indirections.flatten.each do |i| + @routes.delete(i.to_sym) + end end def listening? - @listening + @listening end def listen - raise "Cannot listen -- already listening." if listening? - http_server.listen(@routes.dup) - @listening = true + raise "Cannot listen -- already listening." if listening? + http_server.listen(@routes.dup) + @listening = true end def unlisten - raise "Cannot unlisten -- not currently listening." unless listening? - http_server.unlisten - @listening = false + raise "Cannot unlisten -- not currently listening." unless listening? + http_server.unlisten + @listening = false end def http_server_class -- cgit From 13c40e93a5f65255dd3cf93955e83121cc5bb594 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 19 Feb 2008 13:34:55 -0600 Subject: removing obsolete TODO comment --- lib/puppet/network/http/mongrel.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb index 8ea669531..4593f6569 100644 --- a/lib/puppet/network/http/mongrel.rb +++ b/lib/puppet/network/http/mongrel.rb @@ -45,7 +45,6 @@ class Puppet::Network::HTTP::Mongrel end end - # TODO/FIXME: need a spec which forces delegation to the real class def class_for_protocol(protocol) return Puppet::Network::HTTP::MongrelREST if protocol.to_sym == :rest return Puppet::Network::HTTP::MongrelXMLRPC if protocol.to_sym == :xmlrpc -- cgit From c2f8c69af368a8ba496da4ef0023ac5f0885e3c0 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 19 Feb 2008 15:06:00 -0600 Subject: the indirector will not serve xmlrpc (this is the responsibility of the legacy networking code; it was a mistake to include stubbed support for it in the new code); removing --- lib/puppet/network/http/mongrel.rb | 2 -- lib/puppet/network/http/mongrel/xmlrpc.rb | 4 ---- lib/puppet/network/http/webrick.rb | 2 -- lib/puppet/network/http/webrick/xmlrpc.rb | 4 ---- spec/spec.opts | 2 ++ spec/unit/network/http/mongrel.rb | 9 ++------- spec/unit/network/http/webrick.rb | 9 ++------- 7 files changed, 6 insertions(+), 26 deletions(-) delete mode 100644 lib/puppet/network/http/mongrel/xmlrpc.rb delete mode 100644 lib/puppet/network/http/webrick/xmlrpc.rb diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb index 4593f6569..d948836cd 100644 --- a/lib/puppet/network/http/mongrel.rb +++ b/lib/puppet/network/http/mongrel.rb @@ -1,7 +1,6 @@ require 'mongrel' if Puppet.features.mongrel? require 'puppet/network/http/mongrel/rest' -require 'puppet/network/http/mongrel/xmlrpc' class Puppet::Network::HTTP::Mongrel def initialize(args = {}) @@ -47,7 +46,6 @@ class Puppet::Network::HTTP::Mongrel def class_for_protocol(protocol) return Puppet::Network::HTTP::MongrelREST if protocol.to_sym == :rest - return Puppet::Network::HTTP::MongrelXMLRPC if protocol.to_sym == :xmlrpc raise ArgumentError, "Unknown protocol [#{protocol}]." end end diff --git a/lib/puppet/network/http/mongrel/xmlrpc.rb b/lib/puppet/network/http/mongrel/xmlrpc.rb deleted file mode 100644 index 92acd4f0e..000000000 --- a/lib/puppet/network/http/mongrel/xmlrpc.rb +++ /dev/null @@ -1,4 +0,0 @@ -class Puppet::Network::HTTP::MongrelXMLRPC - def initialize(args = {}) - end -end diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb index c4b2ed3c6..894e12473 100644 --- a/lib/puppet/network/http/webrick.rb +++ b/lib/puppet/network/http/webrick.rb @@ -1,7 +1,6 @@ require 'webrick' require 'webrick/https' require 'puppet/network/http/webrick/rest' -require 'puppet/network/http/webrick/xmlrpc' class Puppet::Network::HTTP::WEBrick def initialize(args = {}) @@ -45,7 +44,6 @@ class Puppet::Network::HTTP::WEBrick def class_for_protocol(protocol) return Puppet::Network::HTTP::WEBrickREST if protocol.to_sym == :rest - return Puppet::Network::HTTP::WEBrickXMLRPC if protocol.to_sym == :xmlrpc raise ArgumentError, "Unknown protocol [#{protocol}]." end end diff --git a/lib/puppet/network/http/webrick/xmlrpc.rb b/lib/puppet/network/http/webrick/xmlrpc.rb deleted file mode 100644 index 793708f8a..000000000 --- a/lib/puppet/network/http/webrick/xmlrpc.rb +++ /dev/null @@ -1,4 +0,0 @@ -class Puppet::Network::HTTP::WEBrickXMLRPC - def initialize(args = {}) - end -end diff --git a/spec/spec.opts b/spec/spec.opts index 2f9bf0d0a..695852c2f 100644 --- a/spec/spec.opts +++ b/spec/spec.opts @@ -1,3 +1,5 @@ +--format +s --colour --loadby mtime diff --git a/spec/unit/network/http/mongrel.rb b/spec/unit/network/http/mongrel.rb index a39e7354a..a837c9daa 100644 --- a/spec/unit/network/http/mongrel.rb +++ b/spec/unit/network/http/mongrel.rb @@ -23,7 +23,7 @@ describe Puppet::Network::HTTP::Mongrel, "when turning on listening" do @mock_mongrel.stubs(:run) @mock_mongrel.stubs(:register) Mongrel::HttpServer.stubs(:new).returns(@mock_mongrel) - @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest, :xmlrpc ] } + @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest ] } end it "should fail if already listening" do @@ -82,11 +82,6 @@ describe Puppet::Network::HTTP::Mongrel, "when turning on listening" do @server.listen(@listen_params.merge(:protocols => [:rest])) end - it "should use a Mongrel + XMLRPC class to configure Mongrel when XMLRPC services are requested" do - Puppet::Network::HTTP::MongrelXMLRPC.expects(:new).at_least_once - @server.listen(@listen_params.merge(:protocols => [:xmlrpc])) - end - it "should fail if services from an unknown protocol are requested" do Proc.new { @server.listen(@listen_params.merge(:protocols => [ :foo ]))}.should raise_error(ArgumentError) end @@ -102,7 +97,7 @@ describe Puppet::Network::HTTP::Mongrel, "when turning off listening" do @mock_mongrel.stubs(:register) Mongrel::HttpServer.stubs(:new).returns(@mock_mongrel) @server = Puppet::Network::HTTP::Mongrel.new - @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest, :xmlrpc ] } + @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest ] } end it "should fail unless listening" do diff --git a/spec/unit/network/http/webrick.rb b/spec/unit/network/http/webrick.rb index 0689b1b6b..6a4c50142 100644 --- a/spec/unit/network/http/webrick.rb +++ b/spec/unit/network/http/webrick.rb @@ -18,7 +18,7 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do [:mount, :start, :shutdown].each {|meth| @mock_webrick.stubs(meth)} WEBrick::HTTPServer.stubs(:new).returns(@mock_webrick) @server = Puppet::Network::HTTP::WEBrick.new - @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest, :xmlrpc ] } + @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest ] } end it "should fail if already listening" do @@ -78,11 +78,6 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do @server.listen(@listen_params.merge(:protocols => [:rest])) end - it "should use a WEBrick + XMLRPC class to configure WEBrick when XMLRPC services are requested" do - Puppet::Network::HTTP::WEBrickXMLRPC.expects(:new).at_least_once - @server.listen(@listen_params.merge(:protocols => [:xmlrpc])) - end - it "should fail if services from an unknown protocol are requested" do Proc.new { @server.listen(@listen_params.merge(:protocols => [ :foo ]))}.should raise_error(ArgumentError) end @@ -94,7 +89,7 @@ describe Puppet::Network::HTTP::WEBrick, "when turning off listening" do [:mount, :start, :shutdown].each {|meth| @mock_webrick.stubs(meth)} WEBrick::HTTPServer.stubs(:new).returns(@mock_webrick) @server = Puppet::Network::HTTP::WEBrick.new - @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest, :xmlrpc ] } + @listen_params = { :address => "127.0.0.1", :port => 31337, :handlers => [ :node, :catalog ], :protocols => [ :rest ] } end it "should fail unless listening" do -- cgit From e86fde2facafd56ee12d7e748b1c8cad916253bf Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 12 Mar 2008 22:26:12 -0500 Subject: This is the first version where mongrel and webrick are reliably startable and stoppable via Puppet::Network::Server. Added a network/server integration spec, testing startup, shutdown, reachability, and collision of webrick and mongrel servers in the new network code. Converted Puppet::Network::HTTP::Handler class to a module, as mongrel Handler should be subclassed; converting subclasses to include the module instead. Mongrel will actually stop if you .stop it, graceful_shutdown didn't seem quite so reliable. Webrick requires running in its own Thread to avoid hanging the entire process; this requires introduction of a Mutex to make things safe. We're only supporting the REST protocol. Made this explicit. Fixed http server setup args, w/ specs, ah the glory of integration testing. --- lib/puppet/network/http/handler.rb | 5 +- lib/puppet/network/http/mongrel.rb | 10 ++-- lib/puppet/network/http/mongrel/rest.rb | 13 ++++- lib/puppet/network/http/webrick.rb | 25 +++++++--- lib/puppet/network/http/webrick/rest.rb | 4 +- lib/puppet/network/server.rb | 6 ++- spec/integration/network/server.rb | 85 +++++++++++++++++++++++++++++++++ spec/unit/network/http/mongrel.rb | 4 +- spec/unit/network/server.rb | 53 ++++++++++++++++---- 9 files changed, 175 insertions(+), 30 deletions(-) create mode 100644 spec/integration/network/server.rb diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 773381c8d..7679bf320 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -1,4 +1,5 @@ -class Puppet::Network::HTTP::Handler +module Puppet::Network::HTTP::Handler + def initialize(args = {}) raise ArgumentError unless @server = args[:server] raise ArgumentError unless @handler = args[:handler] @@ -77,7 +78,7 @@ class Puppet::Network::HTTP::Handler %r{/#{@handler.to_s}s$}.match(path(request)) end - # methods specific to a given web server + # methods to be overridden by the including web server class def register_handler raise NotImplementedError diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb index d948836cd..941ef0e43 100644 --- a/lib/puppet/network/http/mongrel.rb +++ b/lib/puppet/network/http/mongrel.rb @@ -13,20 +13,20 @@ class Puppet::Network::HTTP::Mongrel raise ArgumentError, ":address must be specified." unless args[:address] raise ArgumentError, ":port must be specified." unless args[:port] raise "Mongrel server is already listening" if listening? - + @protocols = args[:protocols] @handlers = args[:handlers] - @server = Mongrel::HttpServer.new(args[:address], args[:port]) - + @server = Mongrel::HttpServer.new(args[:address], args[:port]) setup_handlers - @server.run @listening = true + @server.run end def unlisten raise "Mongrel server is not listening" unless listening? - @server.graceful_shutdown + @server.stop + @server = nil @listening = false end diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index 6c24e360c..7cb6f67bf 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -1,36 +1,45 @@ require 'puppet/network/http/handler' -class Puppet::Network::HTTP::MongrelREST < Puppet::Network::HTTP::Handler +class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler + + include Puppet::Network::HTTP::Handler private - + + # have this mongrel @server listen for /foo and /foos REST endpoints def register_handler @server.register('/' + @handler.to_s, self) @server.register('/' + @handler.to_s + 's', self) end + # which HTTP verb was used in this request def http_method(request) request.params[Mongrel::Const::REQUEST_METHOD] end + # what path was requested? def path(request) # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] x = '/' + request.params[Mongrel::Const::REQUEST_PATH].split('/')[1] end + # return the key included in the request path def request_key(request) # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] x = request.params[Mongrel::Const::REQUEST_PATH].split('/')[2] end + # return the request body def body(request) request.body end + # return the query params for this request def params(request) Mongrel::HttpRequest.query_parse(request.params["QUERY_STRING"]) end + # produce the body of the response def encode_result(request, response, result, status = 200) response.start(status) do |head, body| body.write(result) diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb index 894e12473..3fd643612 100644 --- a/lib/puppet/network/http/webrick.rb +++ b/lib/puppet/network/http/webrick.rb @@ -1,10 +1,12 @@ require 'webrick' require 'webrick/https' require 'puppet/network/http/webrick/rest' +require 'thread' class Puppet::Network::HTTP::WEBrick def initialize(args = {}) @listening = false + @mutex = Mutex.new end def listen(args = {}) @@ -12,24 +14,33 @@ class Puppet::Network::HTTP::WEBrick raise ArgumentError, ":protocols must be specified." if !args[:protocols] or args[:protocols].empty? raise ArgumentError, ":address must be specified." unless args[:address] raise ArgumentError, ":port must be specified." unless args[:port] - raise "WEBrick server is already listening" if listening? @protocols = args[:protocols] @handlers = args[:handlers] @server = WEBrick::HTTPServer.new(:BindAddress => args[:address], :Port => args[:port]) setup_handlers - @server.start - @listening = true + + @mutex.synchronize do + raise "WEBrick server is already listening" if @listening + @listening = true + @thread = Thread.new { @server.start } + end end def unlisten - raise "WEBrick server is not listening" unless listening? - @server.shutdown - @listening = false + @mutex.synchronize do + raise "WEBrick server is not listening" unless @listening + @server.shutdown + @thread.join + @server = nil + @listening = false + end end def listening? - @listening + @mutex.synchronize do + @listening + end end private diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb index 8cda079e2..923e002e3 100644 --- a/lib/puppet/network/http/webrick/rest.rb +++ b/lib/puppet/network/http/webrick/rest.rb @@ -1,6 +1,8 @@ require 'puppet/network/http/handler' -class Puppet::Network::HTTP::WEBrickREST < Puppet::Network::HTTP::Handler +class Puppet::Network::HTTP::WEBrickREST + + include Puppet::Network::HTTP::Handler # WEBrick uses a service() method to respond to requests. Simply delegate to the handler response() method. def service(request, response) diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb index 9e6b82469..cab14519b 100644 --- a/lib/puppet/network/server.rb +++ b/lib/puppet/network/server.rb @@ -1,3 +1,5 @@ +require 'puppet/network/http' + class Puppet::Network::Server attr_reader :server_type, :protocols, :address, :port @@ -8,7 +10,7 @@ class Puppet::Network::Server raise(ArgumentError, "Must specify :address or configure Puppet :bindaddress.") @port = args[:port] || Puppet[:masterport] || raise(ArgumentError, "Must specify :port or configure Puppet :masterport") - @protocols = [] + @protocols = [ :rest ] @listening = false @routes = {} self.register(args[:handlers]) if args[:handlers] @@ -38,8 +40,8 @@ class Puppet::Network::Server def listen raise "Cannot listen -- already listening." if listening? - http_server.listen(@routes.dup) @listening = true + http_server.listen(:address => address, :port => port, :handlers => @routes.keys, :protocols => protocols) end def unlisten diff --git a/spec/integration/network/server.rb b/spec/integration/network/server.rb new file mode 100644 index 000000000..932161b08 --- /dev/null +++ b/spec/integration/network/server.rb @@ -0,0 +1,85 @@ +require File.dirname(__FILE__) + '/../../spec_helper' +require 'puppet/network/server' +require 'socket' + +describe Puppet::Network::Server do + describe "when using webrick" do + before :each do + Puppet[:servertype] = 'webrick' + @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :node ] } + end + + describe "before listening" do + it "should not be reachable at the specified address and port" do + lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error + end + end + + describe "when listening" do + it "should be reachable on the specified address and port" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + lambda { TCPSocket.new('127.0.0.1', 34343) }.should_not raise_error + end + + it "should not allow multiple servers to listen on the same address and port" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + @server2 = Puppet::Network::Server.new(@params.merge(:port => 34343)) + lambda { @server2.listen }.should raise_error + end + + after :each do + @server.unlisten if @server.listening? + end + end + + describe "after unlistening" do + it "should not be reachable on the port and address assigned" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + @server.unlisten + lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error(Errno::ECONNREFUSED) + end + end + end + + describe "when using mongrel" do + before :each do + Puppet[:servertype] = 'mongrel' + @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :node ] } + @server = Puppet::Network::Server.new(@params) + end + + describe "before listening" do + it "should not be reachable at the specified address and port" do + lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) + end + end + + describe "when listening" do + it "should be reachable on the specified address and port" do + @server.listen + lambda { TCPSocket.new('127.0.0.1', 34346) }.should_not raise_error + end + + it "should not allow multiple servers to listen on the same address and port" do + @server.listen + @server2 = Puppet::Network::Server.new(@params) + lambda { @server2.listen }.should raise_error + end + end + + describe "after unlistening" do + it "should not be reachable on the port and address assigned" do + @server.listen + @server.unlisten + lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) + end + end + + after :each do + @server.unlisten if @server.listening? + end + end +end \ No newline at end of file diff --git a/spec/unit/network/http/mongrel.rb b/spec/unit/network/http/mongrel.rb index a837c9daa..cd23ed9e1 100644 --- a/spec/unit/network/http/mongrel.rb +++ b/spec/unit/network/http/mongrel.rb @@ -106,13 +106,13 @@ describe Puppet::Network::HTTP::Mongrel, "when turning off listening" do it "should order mongrel server to stop" do @server.listen(@listen_params) - @mock_mongrel.expects(:graceful_shutdown) + @mock_mongrel.expects(:stop) @server.unlisten end it "should not be listening" do @server.listen(@listen_params) - @mock_mongrel.stubs(:graceful_shutdown) + @mock_mongrel.stubs(:stop) @server.unlisten @server.should_not be_listening end diff --git a/spec/unit/network/server.rb b/spec/unit/network/server.rb index 3e29807ad..e4afbc3c2 100644 --- a/spec/unit/network/server.rb +++ b/spec/unit/network/server.rb @@ -161,8 +161,12 @@ describe Puppet::Network::Server, "in general" do Proc.new { @server2.unregister(:bar) }.should raise_error(ArgumentError) end - it "should provide a means of determining which style of service is being offered to clients" do - @server.protocols.should == [] + it "should provide a means of determining which protocols are in use" do + @server.should respond_to(:protocols) + end + + it "should only support the REST protocol at this time" do + @server.protocols.should == [ :rest ] end it "should provide a means of determining the listening address" do @@ -230,23 +234,54 @@ describe Puppet::Network::Server, "when listening is being turned on" do @mock_http_server_class = mock('http server class') Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class) Puppet.stubs(:[]).with(:servertype).returns(:suparserver) - @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337) + @server = Puppet::Network::Server.new(:address => "127.0.0.1", :port => 31337, :handlers => [:node]) @mock_http_server = mock('http server') @mock_http_server.stubs(:listen) end - it "should fetch an instance of an HTTP server when listening is turned on" do - mock_http_server_class = mock('http server class') - mock_http_server_class.expects(:new).returns(@mock_http_server) - @server.expects(:http_server_class).returns(mock_http_server_class) + it "should fetch an instance of an HTTP server" do + @server.stubs(:http_server_class).returns(@mock_http_server_class) + @mock_http_server_class.expects(:new).returns(@mock_http_server) @server.listen end - it "should cause the HTTP server to listen when listening is turned on" do + it "should cause the HTTP server to listen" do + @server.stubs(:http_server).returns(@mock_http_server) @mock_http_server.expects(:listen) - @server.expects(:http_server).returns(@mock_http_server) @server.listen end + + it "should pass the listening address to the HTTP server" do + @server.stubs(:http_server).returns(@mock_http_server) + @mock_http_server.expects(:listen).with do |args| + args[:address] == '127.0.0.1' + end + @server.listen + end + + it "should pass the listening port to the HTTP server" do + @server.stubs(:http_server).returns(@mock_http_server) + @mock_http_server.expects(:listen).with do |args| + args[:port] == 31337 + end + @server.listen + end + + it "should pass a list of handlers to the HTTP server" do + @server.stubs(:http_server).returns(@mock_http_server) + @mock_http_server.expects(:listen).with do |args| + args[:handlers] == [ :node ] + end + @server.listen + end + + it "should pass a list of protocols to the HTTP server" do + @server.stubs(:http_server).returns(@mock_http_server) + @mock_http_server.expects(:listen).with do |args| + args[:protocols] == [ :rest ] + end + @server.listen + end end describe Puppet::Network::Server, "when listening is being turned off" do -- cgit From 7d51146e98e4135d933be5b2b227a0ca92f06ef2 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 12 Mar 2008 23:08:13 -0500 Subject: fixing Puppet::Node::REST class name to work with autoloader inflection (Puppet::Node::Rest), so we can do Puppet::Node.terminus_class = :rest --- lib/puppet/indirector/node/rest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/indirector/node/rest.rb b/lib/puppet/indirector/node/rest.rb index c5d2f97fb..d8b75f6e7 100644 --- a/lib/puppet/indirector/node/rest.rb +++ b/lib/puppet/indirector/node/rest.rb @@ -1,7 +1,7 @@ require 'puppet/node' require 'puppet/indirector/rest' -class Puppet::Node::REST < Puppet::Indirector::REST +class Puppet::Node::Rest < Puppet::Indirector::REST desc "This will eventually be a REST-based mechanism for finding nodes. It is currently non-functional." # TODO/FIXME end -- cgit From a1c45790f6cac265a7bac7d63bfb8a3204ed228f Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 12 Mar 2008 23:26:05 -0500 Subject: a trivial integration test to test whether the RESTful indirection terminus has a remote shot at working; will need to be upgraded to actually be useful --- spec/integration/indirector/rest.rb | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 spec/integration/indirector/rest.rb diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb new file mode 100644 index 000000000..a8c6f5609 --- /dev/null +++ b/spec/integration/indirector/rest.rb @@ -0,0 +1,35 @@ +require File.dirname(__FILE__) + '/../../spec_helper' +require 'puppet/network/server' +require 'puppet/indirector' +require 'puppet/indirector/rest' + +class Puppet::TestIndirectedFoo + extend Puppet::Indirector + indirects :test_indirected_foo, :terminus_setting => :test_indirected_foo_terminus + + def initialize(foo) + STDERR.puts "foo!" + end +end + +class Puppet::TestIndirectedFoo::Rest < Puppet::Indirector::REST +end + +describe Puppet::Indirector::REST do + before :each do + Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) + Puppet::TestIndirectedFoo.terminus_class = :rest + Puppet[:servertype] = 'mongrel' + @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :test_indirected_foo ] } + @server = Puppet::Network::Server.new(@params) + @server.listen + end + + it "should not fail to find an instance over REST" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error + end + + after :each do + @server.unlisten + end +end \ No newline at end of file -- cgit From 7a7343458402e493f690633f3cfa78abef316d28 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Mon, 31 Mar 2008 11:59:16 -0500 Subject: Much larger commit than I would like to land at once. This is all REST-related code. Two specs are failing related to how Mongrel is initialized for REST; will fix those shortly. REST indirector now supports find, with deserialization. Network code in indirector now. Will still need to un-hardwire address/port for outbound connections. Will still need to urlencode path parameters. Code for search, destroy, update is coming, should be similar to find. Reworked how the Handler module is used. Needed to be included, rather than inherited. Needed to sidestep initializers for actual web servers (webrick, mongrel), needed to be possible to have handler-including class be used as a class (aka servlet) instead of as an instance. Webrick handler registration is now abstracted to "above" the servlet. Provided a #model method to use instead of @model in handler module. This allows neutering during testing. Brought class_for_protocol up into http/webrick class as a (tested) class method. Integration tests for rest indirection. Split server integration tests into mongrel and webrick tests. Got Node/REST working properly wrt the crazy-ass autoloader thing. We're now actually passing traffic w/ webrick, fwiw. --- lib/puppet/indirector/rest.rb | 12 ++++- lib/puppet/network/http/handler.rb | 17 +++--- lib/puppet/network/http/mongrel/rest.rb | 4 ++ lib/puppet/network/http/webrick.rb | 16 +++--- lib/puppet/network/http/webrick/rest.rb | 17 +++--- spec/integration/indirector/rest.rb | 74 ++++++++++++++++++++++---- spec/integration/network/server.rb | 85 ------------------------------ spec/integration/network/server/mongrel.rb | 44 ++++++++++++++++ spec/integration/network/server/webrick.rb | 46 ++++++++++++++++ spec/unit/indirector/node/rest.rb | 4 +- spec/unit/indirector/rest.rb | 61 ++++++++++++++++++--- spec/unit/network/http/webrick.rb | 32 ++++++++--- spec/unit/network/http/webrick/rest.rb | 33 ++++-------- spec/unit/network/server.rb | 6 --- 14 files changed, 287 insertions(+), 164 deletions(-) delete mode 100644 spec/integration/network/server.rb create mode 100644 spec/integration/network/server/mongrel.rb create mode 100644 spec/integration/network/server/webrick.rb diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 7b7c932c4..4c54183c6 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -1,8 +1,16 @@ -require 'puppet/indirector/rest' +require 'net/http' +require 'uri' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus + def network_fetch(path) + # TODO: url_encode path, set proper server + port + Net::HTTP.get(URI.parse("http://127.0.0.1:34343/#{path}")) + end + def find(name, options = {}) - indirection.model.new(name) + network_result = network_fetch("#{indirection.name}/#{name}") + raise YAML.load(network_result) if network_result =~ %r{--- !ruby/exception} + decoded_result = indirection.model.from_yaml(network_result) end end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 7679bf320..b5dcf69d4 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -1,10 +1,9 @@ module Puppet::Network::HTTP::Handler - - def initialize(args = {}) + + def initialize_for_puppet(args = {}) raise ArgumentError unless @server = args[:server] raise ArgumentError unless @handler = args[:handler] @model = find_model_for_handler(@handler) - register_handler end # handle an HTTP request @@ -19,24 +18,28 @@ module Puppet::Network::HTTP::Handler end private + + def model + @model + end def do_find(request, response) key = request_key(request) || raise(ArgumentError, "Could not locate lookup key in request path [#{path}]") args = params(request) - result = @model.find(key, args).to_yaml + result = model.find(key, args).to_yaml encode_result(request, response, result) end def do_search(request, response) args = params(request) - result = @model.search(args).collect {|obj| obj.to_yaml } + result = model.search(args).collect {|obj| obj.to_yaml } encode_result(request, response, result) end def do_destroy(request, response) key = request_key(request) || raise(ArgumentError, "Could not locate lookup key in request path [#{path}]") args = params(request) - result = @model.destroy(key, args) + result = model.destroy(key, args) encode_result(request, response, YAML.dump(result)) end @@ -44,7 +47,7 @@ module Puppet::Network::HTTP::Handler data = body(request) raise ArgumentError, "No data to save" if !data or data.empty? args = params(request) - obj = @model.new + obj = model.new result = obj.save(args.merge(:data => data)).to_yaml encode_result(request, response, result) end diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index 7cb6f67bf..d7a1a1bdc 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -3,6 +3,10 @@ require 'puppet/network/http/handler' class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler include Puppet::Network::HTTP::Handler + + def initialize(args={}) + initialize_for_puppet(args) + end private diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb index 3fd643612..3a37e2071 100644 --- a/lib/puppet/network/http/webrick.rb +++ b/lib/puppet/network/http/webrick.rb @@ -9,6 +9,11 @@ class Puppet::Network::HTTP::WEBrick @mutex = Mutex.new end + def self.class_for_protocol(protocol) + return Puppet::Network::HTTP::WEBrickREST if protocol.to_sym == :rest + raise "Unknown protocol [#{protocol}]." + end + def listen(args = {}) raise ArgumentError, ":handlers must be specified." if !args[:handlers] or args[:handlers].empty? raise ArgumentError, ":protocols must be specified." if !args[:protocols] or args[:protocols].empty? @@ -42,19 +47,16 @@ class Puppet::Network::HTTP::WEBrick @listening end end - + private def setup_handlers @protocols.each do |protocol| + klass = self.class.class_for_protocol(protocol) @handlers.each do |handler| - class_for_protocol(protocol).new(:server => @server, :handler => handler) + @server.mount('/' + handler.to_s, klass, handler) + @server.mount('/' + handler.to_s + 's', klass, handler) end end end - - def class_for_protocol(protocol) - return Puppet::Network::HTTP::WEBrickREST if protocol.to_sym == :rest - raise ArgumentError, "Unknown protocol [#{protocol}]." - end end diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb index 923e002e3..b43912196 100644 --- a/lib/puppet/network/http/webrick/rest.rb +++ b/lib/puppet/network/http/webrick/rest.rb @@ -1,21 +1,22 @@ require 'puppet/network/http/handler' -class Puppet::Network::HTTP::WEBrickREST - +class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet + include Puppet::Network::HTTP::Handler + + def initialize(server, handler) + raise ArgumentError, "server is required" unless server + super(server) + initialize_for_puppet(:server => server, :handler => handler) + end # WEBrick uses a service() method to respond to requests. Simply delegate to the handler response() method. def service(request, response) process(request, response) end - + private - def register_handler - @server.mount('/' + @handler.to_s, self) - @server.mount('/' + @handler.to_s + 's', self) - end - def http_method(request) request.request_method end diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index a8c6f5609..aed17d977 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -3,32 +3,88 @@ require 'puppet/network/server' require 'puppet/indirector' require 'puppet/indirector/rest' +# a fake class that will be indirected via REST class Puppet::TestIndirectedFoo extend Puppet::Indirector indirects :test_indirected_foo, :terminus_setting => :test_indirected_foo_terminus - def initialize(foo) - STDERR.puts "foo!" + attr_reader :value + + def initialize(value = 0) + @value = value + end + + def self.from_yaml(yaml) + YAML.load(yaml) end end +# empty Terminus class -- this would normally have to be in a directory findable by the autoloader, but we short-circuit that below class Puppet::TestIndirectedFoo::Rest < Puppet::Indirector::REST end + describe Puppet::Indirector::REST do before :each do - Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) - Puppet::TestIndirectedFoo.terminus_class = :rest - Puppet[:servertype] = 'mongrel' - @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :test_indirected_foo ] } + Puppet[:servertype] = 'webrick' + @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :test_indirected_foo ] } @server = Puppet::Network::Server.new(@params) @server.listen + + # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. + Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) + Puppet::TestIndirectedFoo.terminus_class = :rest end - it "should not fail to find an instance over REST" do - lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error - end + describe "when finding a model instance over REST" do + describe "when a matching model instance can be found" do + before :each do + @model_instance = Puppet::TestIndirectedFoo.new(23) + @mock_model = stub('faked model', :find => @model_instance) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error + end + + it 'should return an instance of the model class' do + Puppet::TestIndirectedFoo.find('bar').class.should == Puppet::TestIndirectedFoo + end + it 'should return the instance of the model class associated with the provided lookup key' do + Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value + end + + it 'should set a version timestamp on model instance' do + Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + end + end + + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :find => nil) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return nil" do + Puppet::TestIndirectedFoo.find('bar').should be_nil + end + end + + describe "when an exception is encountered in looking up a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:find).raises(RuntimeError) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + # it "should raise an exception" do + # lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + # end + end + end + after :each do @server.unlisten end diff --git a/spec/integration/network/server.rb b/spec/integration/network/server.rb deleted file mode 100644 index 932161b08..000000000 --- a/spec/integration/network/server.rb +++ /dev/null @@ -1,85 +0,0 @@ -require File.dirname(__FILE__) + '/../../spec_helper' -require 'puppet/network/server' -require 'socket' - -describe Puppet::Network::Server do - describe "when using webrick" do - before :each do - Puppet[:servertype] = 'webrick' - @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :node ] } - end - - describe "before listening" do - it "should not be reachable at the specified address and port" do - lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error - end - end - - describe "when listening" do - it "should be reachable on the specified address and port" do - @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) - @server.listen - lambda { TCPSocket.new('127.0.0.1', 34343) }.should_not raise_error - end - - it "should not allow multiple servers to listen on the same address and port" do - @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) - @server.listen - @server2 = Puppet::Network::Server.new(@params.merge(:port => 34343)) - lambda { @server2.listen }.should raise_error - end - - after :each do - @server.unlisten if @server.listening? - end - end - - describe "after unlistening" do - it "should not be reachable on the port and address assigned" do - @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) - @server.listen - @server.unlisten - lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error(Errno::ECONNREFUSED) - end - end - end - - describe "when using mongrel" do - before :each do - Puppet[:servertype] = 'mongrel' - @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :node ] } - @server = Puppet::Network::Server.new(@params) - end - - describe "before listening" do - it "should not be reachable at the specified address and port" do - lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) - end - end - - describe "when listening" do - it "should be reachable on the specified address and port" do - @server.listen - lambda { TCPSocket.new('127.0.0.1', 34346) }.should_not raise_error - end - - it "should not allow multiple servers to listen on the same address and port" do - @server.listen - @server2 = Puppet::Network::Server.new(@params) - lambda { @server2.listen }.should raise_error - end - end - - describe "after unlistening" do - it "should not be reachable on the port and address assigned" do - @server.listen - @server.unlisten - lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) - end - end - - after :each do - @server.unlisten if @server.listening? - end - end -end \ No newline at end of file diff --git a/spec/integration/network/server/mongrel.rb b/spec/integration/network/server/mongrel.rb new file mode 100644 index 000000000..7bf8291a3 --- /dev/null +++ b/spec/integration/network/server/mongrel.rb @@ -0,0 +1,44 @@ +require File.dirname(__FILE__) + '/../../../spec_helper' +require 'puppet/network/server' +require 'socket' + +describe Puppet::Network::Server do + describe "when using mongrel" do + before :each do + Puppet[:servertype] = 'mongrel' + @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :node ] } + @server = Puppet::Network::Server.new(@params) + end + + describe "before listening" do + it "should not be reachable at the specified address and port" do + lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) + end + end + + describe "when listening" do + it "should be reachable on the specified address and port" do + @server.listen + lambda { TCPSocket.new('127.0.0.1', 34346) }.should_not raise_error + end + + it "should not allow multiple servers to listen on the same address and port" do + @server.listen + @server2 = Puppet::Network::Server.new(@params) + lambda { @server2.listen }.should raise_error + end + end + + describe "after unlistening" do + it "should not be reachable on the port and address assigned" do + @server.listen + @server.unlisten + lambda { TCPSocket.new('127.0.0.1', 34346) }.should raise_error(Errno::ECONNREFUSED) + end + end + + after :each do + @server.unlisten if @server.listening? + end + end +end \ No newline at end of file diff --git a/spec/integration/network/server/webrick.rb b/spec/integration/network/server/webrick.rb new file mode 100644 index 000000000..0ab8d89e4 --- /dev/null +++ b/spec/integration/network/server/webrick.rb @@ -0,0 +1,46 @@ +require File.dirname(__FILE__) + '/../../../spec_helper' +require 'puppet/network/server' +require 'socket' + +describe Puppet::Network::Server do + describe "when using webrick" do + before :each do + Puppet[:servertype] = 'webrick' + @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :node ] } + end + + describe "before listening" do + it "should not be reachable at the specified address and port" do + lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error + end + end + + describe "when listening" do + it "should be reachable on the specified address and port" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + lambda { TCPSocket.new('127.0.0.1', 34343) }.should_not raise_error + end + + it "should not allow multiple servers to listen on the same address and port" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + @server2 = Puppet::Network::Server.new(@params.merge(:port => 34343)) + lambda { @server2.listen }.should raise_error + end + + after :each do + @server.unlisten if @server.listening? + end + end + + describe "after unlistening" do + it "should not be reachable on the port and address assigned" do + @server = Puppet::Network::Server.new(@params.merge(:port => 34343)) + @server.listen + @server.unlisten + lambda { TCPSocket.new('127.0.0.1', 34343) }.should raise_error(Errno::ECONNREFUSED) + end + end + end +end \ No newline at end of file diff --git a/spec/unit/indirector/node/rest.rb b/spec/unit/indirector/node/rest.rb index 33cfda426..3b99e33c4 100755 --- a/spec/unit/indirector/node/rest.rb +++ b/spec/unit/indirector/node/rest.rb @@ -4,9 +4,9 @@ require File.dirname(__FILE__) + '/../../../spec_helper' require 'puppet/indirector/node/rest' -describe Puppet::Node::REST do +describe Puppet::Node::Rest do before do - @searcher = Puppet::Node::REST.new + @searcher = Puppet::Node::Rest.new end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 3c4716dc8..56db8f509 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -4,7 +4,6 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/indirector/rest' describe Puppet::Indirector::REST do - # FIXME : TODO / look through this, does this make sense? before do Puppet::Indirector::Terminus.stubs(:register_terminus_class) @model = mock 'model' @@ -20,11 +19,57 @@ describe Puppet::Indirector::REST do @searcher = @rest_class.new end - it "should return an instance of the indirected model" - it "should deserialize result data after a call into a Model instance for find" - it "should deserialize result data after a call into a list of Model instances for search" - it "should deserialize result data after a call into a boolean for save" - it "should deserialize result data after a call into a boolean for destroy" - it "should generate an error when result data deserializes improperly" - it "should generate an error when result data specifies an error" + describe "when doing a find" do + before :each do + @searcher.stubs(:network_fetch).returns({:foo => 'bar'}.to_yaml) # neuter the network connection + @model.stubs(:from_yaml).returns(@instance) + end + + it "should look up the model instance over the network" do + @searcher.expects(:network_fetch).returns({:foo => 'bar'}.to_yaml) + @searcher.find('foo') + end + + it "should deserialize result data to a Model instance" do + @model.expects(:from_yaml) + @searcher.find('foo') + end + + it "should return the deserialized Model instance" do + @searcher.find('foo').should == @instance + end + + it "should return nil when deserialized model instance is nil" do + @model.stubs(:from_yaml).returns(@instance) + @searcher.find('foo').should be_nil + end + + it "should generate an error when result data deserializes improperly" do + @model.stubs(:from_yaml).raises(ArgumentError) + lambda { @searcher.find('foo') }.should raise_error(ArgumentError) + end + + it "should generate an error when result data specifies an error" do + @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml) + lambda { @searcher.find('foo') }.should raise_error(RuntimeError) + end + end + + describe "when doing a search" do + it "should deserialize result data into a list of Model instances" + it "should generate an error when result data deserializes improperly" + it "should generate an error when result data specifies an error" + end + + describe "when doing a save" do + it "should deserialize result data into a boolean" + it "should generate an error when result data deserializes improperly" + it "should generate an error when result data specifies an error" + end + + describe "when doing a destroy" do + it "should deserialize result data into a boolean" + it "should generate an error when result data deserializes improperly" + it "should generate an error when result data specifies an error" + end end diff --git a/spec/unit/network/http/webrick.rb b/spec/unit/network/http/webrick.rb index 6a4c50142..05ed2f0e2 100644 --- a/spec/unit/network/http/webrick.rb +++ b/spec/unit/network/http/webrick.rb @@ -14,7 +14,7 @@ end describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do before do - @mock_webrick = mock('webrick') + @mock_webrick = stub('webrick', :[] => {}) [:mount, :start, :shutdown].each {|meth| @mock_webrick.stubs(meth)} WEBrick::HTTPServer.stubs(:new).returns(@mock_webrick) @server = Puppet::Network::HTTP::WEBrick.new @@ -64,28 +64,44 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do mock_handler = mock("handler instance for [#{protocol}]") mock_handler_class = mock("handler class for [#{protocol}]") @listen_params[:handlers].each do |handler| - mock_handler_class.expects(:new).with {|args| - args[:server] == @mock_webrick and args[:handler] == handler - }.returns(mock_handler) + @mock_webrick.expects(:mount) end - @server.expects(:class_for_protocol).with(protocol).at_least_once.returns(mock_handler_class) end @server.listen(@listen_params) end it "should use a WEBrick + REST class to configure WEBrick when REST services are requested" do - Puppet::Network::HTTP::WEBrickREST.expects(:new).at_least_once + Puppet::Network::HTTP::WEBrick.expects(:class_for_protocol).with(:rest).at_least_once @server.listen(@listen_params.merge(:protocols => [:rest])) end it "should fail if services from an unknown protocol are requested" do - Proc.new { @server.listen(@listen_params.merge(:protocols => [ :foo ]))}.should raise_error(ArgumentError) + Proc.new { @server.listen(@listen_params.merge(:protocols => [ :foo ]))}.should raise_error end end + +describe Puppet::Network::HTTP::WEBrick, "when looking up the class to handle a protocol" do + it "should require a protocol" do + lambda { Puppet::Network::HTTP::WEBrick.class_for_protocol }.should raise_error(ArgumentError) + end + + it "should accept a protocol" do + lambda { Puppet::Network::HTTP::WEBrick.class_for_protocol("bob") }.should_not raise_error(ArgumentError) + end + + it "should use a WEBrick + REST class when a REST protocol is specified" do + Puppet::Network::HTTP::WEBrick.class_for_protocol("rest").should == Puppet::Network::HTTP::WEBrickREST + end + + it "should fail when an unknown protocol is specified" do + lambda { Puppet::Network::HTTP::WEBrick.class_for_protocol("abcdefg") }.should raise_error + end +end + describe Puppet::Network::HTTP::WEBrick, "when turning off listening" do before do - @mock_webrick = mock('webrick') + @mock_webrick = stub('webrick', :[] => {}) [:mount, :start, :shutdown].each {|meth| @mock_webrick.stubs(meth)} WEBrick::HTTPServer.stubs(:new).returns(@mock_webrick) @server = Puppet::Network::HTTP::WEBrick.new diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb index aa7a3d53a..cafd38b82 100644 --- a/spec/unit/network/http/webrick/rest.rb +++ b/spec/unit/network/http/webrick/rest.rb @@ -8,42 +8,31 @@ require 'puppet/network/http' describe Puppet::Network::HTTP::WEBrickREST, "when initializing" do before do - @mock_webrick = stub('WEBrick server', :mount => true) + @mock_webrick = stub('WEBrick server', :mount => true, :[] => {}) @mock_model = mock('indirected model') Puppet::Indirector::Indirection.stubs(:model).returns(@mock_model) - @params = { :server => @mock_webrick, :handler => :foo } + @params = [ @mock_webrick, :foo ] end it "should require access to a WEBrick server" do - Proc.new { Puppet::Network::HTTP::WEBrickREST.new(@params.delete_if {|k,v| :server == k })}.should raise_error(ArgumentError) + Proc.new { + @params[0] = nil + Puppet::Network::HTTP::WEBrickREST.new(*@params) + }.should raise_error(ArgumentError) end it "should require an indirection name" do - Proc.new { Puppet::Network::HTTP::WEBrickREST.new(@params.delete_if {|k,v| :handler == k })}.should raise_error(ArgumentError) + Proc.new { Puppet::Network::HTTP::WEBrickREST.new(@params.first) }.should raise_error(ArgumentError) end it "should look up the indirection model from the indirection name" do Puppet::Indirector::Indirection.expects(:model).returns(@mock_model) - Puppet::Network::HTTP::WEBrickREST.new(@params) + Puppet::Network::HTTP::WEBrickREST.new(*@params) end it "should fail if the indirection is not known" do Puppet::Indirector::Indirection.expects(:model).returns(nil) - Proc.new { Puppet::Network::HTTP::WEBrickREST.new(@params) }.should raise_error(ArgumentError) - end - - it "should register itself with the WEBrick server for the singular HTTP methods" do - @mock_webrick.expects(:mount).with do |*args| - args.first == '/foo' and args.last.is_a?(Puppet::Network::HTTP::WEBrickREST) - end - Puppet::Network::HTTP::WEBrickREST.new(@params) - end - - it "should register itself with the WEBrick server for the plural GET method" do - @mock_webrick.expects(:mount).with do |*args| - args.first == '/foos' and args.last.is_a?(Puppet::Network::HTTP::WEBrickREST) - end - Puppet::Network::HTTP::WEBrickREST.new(@params) + Proc.new { Puppet::Network::HTTP::WEBrickREST.new(*@params) }.should raise_error(ArgumentError) end end @@ -52,9 +41,9 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @mock_request = stub('webrick http request', :query => {}) @mock_response = stub('webrick http response', :status= => true, :body= => true) @mock_model_class = stub('indirected model class') - @mock_webrick = stub('mongrel http server', :mount => true) + @mock_webrick = stub('webrick http server', :mount => true, :[] => {}) Puppet::Indirector::Indirection.stubs(:model).with(:foo).returns(@mock_model_class) - @handler = Puppet::Network::HTTP::WEBrickREST.new(:server => @mock_webrick, :handler => :foo) + @handler = Puppet::Network::HTTP::WEBrickREST.new(@mock_webrick, :foo) end def setup_find_request diff --git a/spec/unit/network/server.rb b/spec/unit/network/server.rb index e4afbc3c2..2e02ffbc3 100644 --- a/spec/unit/network/server.rb +++ b/spec/unit/network/server.rb @@ -315,10 +315,4 @@ describe Class.new, "put these somewhere" do it "should allow from_* on the inbound :data packet (look at its content_type) when doing a PUT/.new.save" it "should prepend a rest version number on the path (w00t)" it "should ... on server side, .save should from_yaml, then foo.save(args) instead of just Foo.new.save(args)" - it "should have a from_yaml class_method in the indirector (... default: yaml.load(data) => instance, but can be overridden)" -end - -describe Puppet::Indirector, "stuff required by HTTP servers" do - it "should provide the model with the ability to serialize to XML" - it "should provide the model with the ability to deserialize from XML" end -- cgit From d24c03c9bbcc35a94a8235c030a73233feabad57 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Mon, 31 Mar 2008 12:08:36 -0500 Subject: exceptions on remote end now properly passed to local end via REST and re-raised (integration-tested) --- lib/puppet/network/http/handler.rb | 2 +- spec/integration/indirector/rest.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index b5dcf69d4..f226ae133 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -53,7 +53,7 @@ module Puppet::Network::HTTP::Handler end def do_exception(request, response, exception, status=404) - encode_result(request, response, exception.to_s, status) + encode_result(request, response, exception.to_yaml, status) end def find_model_for_handler(handler) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index aed17d977..9ac2bc997 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -79,9 +79,9 @@ describe Puppet::Indirector::REST do Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) end - # it "should raise an exception" do - # lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) - # end + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + end end end -- cgit From 1e0f19bcbd0c3f5a83f9cad0e90eb5d6478e278b Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 1 Apr 2008 22:55:22 -0500 Subject: Make mongrel happy like WEBrick. Refactored specs to put some of the lower-level find/save/search/destroy unit tests under their own contexts. --- lib/puppet/network/http/mongrel.rb | 4 +- lib/puppet/network/http/mongrel/rest.rb | 7 +- spec/unit/network/http/mongrel.rb | 12 +- spec/unit/network/http/mongrel/rest.rb | 264 +++++++++++++++--------------- spec/unit/network/http/webrick/rest.rb | 274 ++++++++++++++++---------------- 5 files changed, 274 insertions(+), 287 deletions(-) diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb index 941ef0e43..9a4531c7a 100644 --- a/lib/puppet/network/http/mongrel.rb +++ b/lib/puppet/network/http/mongrel.rb @@ -38,8 +38,10 @@ class Puppet::Network::HTTP::Mongrel def setup_handlers @protocols.each do |protocol| + klass = class_for_protocol(protocol) @handlers.each do |handler| - class_for_protocol(protocol).new(:server => @server, :handler => handler) + @server.register('/' + handler.to_s, klass.new(:server => @server, :handler => handler)) + @server.register('/' + handler.to_s + 's', klass.new(:server => @server, :handler => handler)) end end end diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index d7a1a1bdc..2a3d4f143 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -5,17 +5,12 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler include Puppet::Network::HTTP::Handler def initialize(args={}) + super() initialize_for_puppet(args) end private - # have this mongrel @server listen for /foo and /foos REST endpoints - def register_handler - @server.register('/' + @handler.to_s, self) - @server.register('/' + @handler.to_s + 's', self) - end - # which HTTP verb was used in this request def http_method(request) request.params[Mongrel::Const::REQUEST_METHOD] diff --git a/spec/unit/network/http/mongrel.rb b/spec/unit/network/http/mongrel.rb index cd23ed9e1..ccfca2f55 100644 --- a/spec/unit/network/http/mongrel.rb +++ b/spec/unit/network/http/mongrel.rb @@ -65,27 +65,21 @@ describe Puppet::Network::HTTP::Mongrel, "when turning on listening" do it "should instantiate a handler for each protocol+handler pair to configure web server routing" do @listen_params[:protocols].each do |protocol| - mock_handler = mock("handler instance for [#{protocol}]") - mock_handler_class = mock("handler class for [#{protocol}]") @listen_params[:handlers].each do |handler| - mock_handler_class.expects(:new).with {|args| - args[:server] == @mock_mongrel and args[:handler] == handler - }.returns(mock_handler) + @mock_mongrel.expects(:register) end - @server.expects(:class_for_protocol).with(protocol).at_least_once.returns(mock_handler_class) end @server.listen(@listen_params) end it "should use a Mongrel + REST class to configure Mongrel when REST services are requested" do - Puppet::Network::HTTP::MongrelREST.expects(:new).at_least_once - @server.listen(@listen_params.merge(:protocols => [:rest])) + @server.expects(:class_for_protocol).with(:rest).at_least_once.returns(Puppet::Network::HTTP::MongrelREST) + @server.listen(@listen_params) end it "should fail if services from an unknown protocol are requested" do Proc.new { @server.listen(@listen_params.merge(:protocols => [ :foo ]))}.should raise_error(ArgumentError) end - end describe Puppet::Network::HTTP::Mongrel, "when turning off listening" do diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb index 9b2feb6ee..3de314852 100644 --- a/spec/unit/network/http/mongrel/rest.rb +++ b/spec/unit/network/http/mongrel/rest.rb @@ -1,8 +1,3 @@ -#!/usr/bin/env ruby -# -# Created by Rick Bradley on 2007-10-16. -# Copyright (c) 2007. All rights reserved. - require File.dirname(__FILE__) + '/../../../../spec_helper' require 'puppet/network/http' @@ -34,20 +29,6 @@ describe Puppet::Network::HTTP::MongrelREST, "when initializing" do Puppet::Indirector::Indirection.expects(:model).with(:foo).returns(nil) Proc.new { Puppet::Network::HTTP::MongrelREST.new(@params) }.should raise_error(ArgumentError) end - - it "should register itself with the mongrel server for the singular HTTP methods" do - @mock_mongrel.expects(:register).with do |*args| - args.first == '/foo' and args.last.is_a? Puppet::Network::HTTP::MongrelREST - end - Puppet::Network::HTTP::MongrelREST.new(@params) - end - - it "should register itself with the mongrel server for the plural GET method" do - @mock_mongrel.expects(:register).with do |*args| - args.first == '/foos' and args.last.is_a? Puppet::Network::HTTP::MongrelREST - end - Puppet::Network::HTTP::MongrelREST.new(@params) - end end describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do @@ -147,137 +128,146 @@ describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do @handler.process(@mock_request, @mock_response) end - it "should fail to find model if key is not specified" do - @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'GET', Mongrel::Const::REQUEST_PATH => '/foo'}) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should fail to destroy model if key is not specified" do - @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'DELETE', Mongrel::Const::REQUEST_PATH => '/foo'}) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end - it "should fail to save model if data is not specified" do - @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'PUT', Mongrel::Const::REQUEST_PATH => '/foo'}) - @mock_request.stubs(:body).returns('') - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end + describe "when finding a model instance" do |variable| + it "should fail to find model if key is not specified" do + @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'GET', Mongrel::Const::REQUEST_PATH => '/foo'}) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end - it "should pass HTTP request parameters to model find" do - setup_find_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') - @mock_model_class.expects(:find).with do |key, args| - key == 'key' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + it "should pass HTTP request parameters to model find" do + setup_find_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') + @mock_model_class.expects(:find).with do |key, args| + key == 'key' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + end + @handler.process(@mock_request, @mock_response) end - @handler.process(@mock_request, @mock_response) - end - - it "should pass HTTP request parameters to model search" do - setup_search_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') - @mock_model_class.expects(:search).with do |args| - args['foo'] == 'baz' and args['bar'] == 'xyzzy' - end.returns([]) - @handler.process(@mock_request, @mock_response) - end - - it "should pass HTTP request parameters to model delete" do - setup_destroy_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') - @mock_model_class.expects(:destroy).with do |key, args| - key == 'key' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + + it "should generate a 200 response when a model find call succeeds" do + setup_find_request + @mock_response.expects(:start).with(200) + @handler.process(@mock_request, @mock_response) end - @handler.process(@mock_request, @mock_response) - end - - it "should pass HTTP request parameters to model save" do - setup_save_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') - @mock_model_instance.expects(:save).with do |args| - args[:data] == 'this is a fake request body' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + + it "should return a serialized object when a model find call succeeds" do + setup_find_request + @mock_model_instance = stub('model instance') + @mock_model_instance.expects(:to_yaml) + @mock_model_class.stubs(:find).returns(@mock_model_instance) + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by find" do + setup_find_request + @mock_model_class.expects(:find).raises(ArgumentError) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) end - @handler.process(@mock_request, @mock_response) end - it "should generate a 200 response when a model find call succeeds" do - setup_find_request - @mock_response.expects(:start).with(200) - @handler.process(@mock_request, @mock_response) + describe "when destroying a model instance" do |variable| + it "should fail to destroy model if key is not specified" do + @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'DELETE', Mongrel::Const::REQUEST_PATH => '/foo'}) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end + + it "should pass HTTP request parameters to model destroy" do + setup_destroy_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') + @mock_model_class.expects(:destroy).with do |key, args| + key == 'key' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + end + @handler.process(@mock_request, @mock_response) + end + + it "should generate a 200 response when a model destroy call succeeds" do + setup_destroy_request + @mock_response.expects(:start).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a serialized success result when a model destroy call succeeds" do + setup_destroy_request + @mock_model_class.stubs(:destroy).returns(true) + @mock_body.expects(:write).with("--- true\n") + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by destroy" do + setup_destroy_request + @mock_model_class.expects(:destroy).raises(ArgumentError) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end end - it "should generate a 200 response when a model search call succeeds" do - setup_search_request - @mock_response.expects(:start).with(200) - @handler.process(@mock_request, @mock_response) + describe "when saving a model instance" do |variable| + it "should fail to save model if data is not specified" do + @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'PUT', Mongrel::Const::REQUEST_PATH => '/foo'}) + @mock_request.stubs(:body).returns('') + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end + + it "should pass HTTP request parameters to model save" do + setup_save_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') + @mock_model_instance.expects(:save).with do |args| + args[:data] == 'this is a fake request body' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' + end + @handler.process(@mock_request, @mock_response) + end + + it "should generate a 200 response when a model save call succeeds" do + setup_save_request + @mock_response.expects(:start).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a serialized object when a model save call succeeds" do + setup_save_request + @mock_model_instance.stubs(:save).returns(@mock_model_instance) + @mock_model_instance.expects(:to_yaml).returns('foo') + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by save" do + setup_save_request + @mock_model_instance.expects(:save).raises(ArgumentError) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end end - it "should generate a 200 response when a model destroy call succeeds" do - setup_destroy_request - @mock_response.expects(:start).with(200) - @handler.process(@mock_request, @mock_response) - end + describe "when searching for model instances" do |variable| + it "should pass HTTP request parameters to model search" do + setup_search_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') + @mock_model_class.expects(:search).with do |args| + args['foo'] == 'baz' and args['bar'] == 'xyzzy' + end.returns([]) + @handler.process(@mock_request, @mock_response) + end - it "should generate a 200 response when a model save call succeeds" do - setup_save_request - @mock_response.expects(:start).with(200) - @handler.process(@mock_request, @mock_response) - end - - it "should return a serialized object when a model find call succeeds" do - setup_find_request - @mock_model_instance = stub('model instance') - @mock_model_instance.expects(:to_yaml) - @mock_model_class.stubs(:find).returns(@mock_model_instance) - @handler.process(@mock_request, @mock_response) - end - - it "should return a list of serialized objects when a model search call succeeds" do - setup_search_request - mock_matches = [1..5].collect {|i| mock("model instance #{i}", :to_yaml => "model instance #{i}") } - @mock_model_class.stubs(:search).returns(mock_matches) - @handler.process(@mock_request, @mock_response) - end - - it "should return a serialized success result when a model destroy call succeeds" do - setup_destroy_request - @mock_model_class.stubs(:destroy).returns(true) - @mock_body.expects(:write).with("--- true\n") - @handler.process(@mock_request, @mock_response) - end - - it "should return a serialized object when a model save call succeeds" do - setup_save_request - @mock_model_instance.stubs(:save).returns(@mock_model_instance) - @mock_model_instance.expects(:to_yaml).returns('foo') - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by find" do - setup_find_request - @mock_model_class.expects(:find).raises(ArgumentError) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end + it "should generate a 200 response when a model search call succeeds" do + setup_search_request + @mock_response.expects(:start).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a list of serialized objects when a model search call succeeds" do + setup_search_request + mock_matches = [1..5].collect {|i| mock("model instance #{i}", :to_yaml => "model instance #{i}") } + @mock_model_class.stubs(:search).returns(mock_matches) + @handler.process(@mock_request, @mock_response) + end - it "should serialize a controller exception when an exception is thrown by search" do - setup_search_request - @mock_model_class.expects(:search).raises(ArgumentError) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by destroy" do - setup_destroy_request - @mock_model_class.expects(:destroy).raises(ArgumentError) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by save" do - setup_save_request - @mock_model_instance.expects(:save).raises(ArgumentError) - @mock_response.expects(:start).with(404) - @handler.process(@mock_request, @mock_response) - end + it "should serialize a controller exception when an exception is thrown by search" do + setup_search_request + @mock_model_class.expects(:search).raises(ArgumentError) + @mock_response.expects(:start).with(404) + @handler.process(@mock_request, @mock_response) + end + end it "should serialize a controller exception if the request fails" do setup_bad_request diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb index cafd38b82..cdc1164fc 100644 --- a/spec/unit/network/http/webrick/rest.rb +++ b/spec/unit/network/http/webrick/rest.rb @@ -1,8 +1,3 @@ -#!/usr/bin/env ruby -# -# Created by Rick Bradley on 2007-10-16. -# Copyright (c) 2007. All rights reserved. - require File.dirname(__FILE__) + '/../../../../spec_helper' require 'puppet/network/http' @@ -77,6 +72,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @mock_request.stubs(:path).returns('/foos') end + it "should call the model find method if the request represents a singular HTTP GET" do setup_find_request @mock_model_class.expects(:find).with('key', {}) @@ -109,12 +105,14 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @handler.process(@mock_request, @mock_response) end - it "should fail if the request's pluralization is wrong" do + it "should fail if delete request's pluralization is wrong" do @mock_request.stubs(:request_method).returns('DELETE') @mock_request.stubs(:path).returns('/foos/key') @mock_response.expects(:status=).with(404) @handler.process(@mock_request, @mock_response) - + end + + it "should fail if put request's pluralization is wrong" do @mock_request.stubs(:request_method).returns('PUT') @mock_request.stubs(:path).returns('/foos/key') @mock_response.expects(:status=).with(404) @@ -128,145 +126,153 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @handler.process(@mock_request, @mock_response) end - it "should fail to find model if key is not specified" do - @mock_request.stubs(:request_method).returns('GET') - @mock_request.stubs(:path).returns('/foo') - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should fail to destroy model if key is not specified" do - @mock_request.stubs(:request_method).returns('DELETE') - @mock_request.stubs(:path).returns('/foo') - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should fail to save model if data is not specified" do - @mock_request.stubs(:request_method).returns('PUT') - @mock_request.stubs(:path).returns('/foo') - @mock_request.stubs(:body).returns('') - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should pass HTTP request parameters to model find" do - setup_find_request - @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) - @mock_model_class.expects(:find).with do |key, args| - key == 'key' and args[:foo] == :baz and args[:bar] == :xyzzy + describe "when finding a model instance" do |variable| + it "should fail to find model if key is not specified" do + @mock_request.stubs(:request_method).returns('GET') + @mock_request.stubs(:path).returns('/foo') + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) + end + + it "should pass HTTP request parameters to model find" do + setup_find_request + @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) + @mock_model_class.expects(:find).with do |key, args| + key == 'key' and args[:foo] == :baz and args[:bar] == :xyzzy + end + @handler.service(@mock_request, @mock_response) + end + + it "should generate a 200 response when a model find call succeeds" do + setup_find_request + @mock_response.expects(:status=).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a serialized object when a model find call succeeds" do + setup_find_request + @mock_model_instance = stub('model instance') + @mock_model_instance.expects(:to_yaml) + @mock_model_class.stubs(:find).returns(@mock_model_instance) + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by find" do + setup_find_request + @mock_model_class.expects(:find).raises(ArgumentError) + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) end - @handler.service(@mock_request, @mock_response) - end - - it "should pass HTTP request parameters to model search" do - setup_search_request - @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) - @mock_model_class.expects(:search).with do |args| - args[:foo] == :baz and args[:bar] == :xyzzy - end.returns([]) - @handler.service(@mock_request, @mock_response) end - it "should pass HTTP request parameters to model destroy" do - setup_destroy_request - @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) - @mock_model_class.expects(:destroy).with do |key, args| - key == 'key' and args[:foo] == :baz and args[:bar] == :xyzzy + describe "when destroying a model instance" do |variable| + it "should fail to destroy model if key is not specified" do + @mock_request.stubs(:request_method).returns('DELETE') + @mock_request.stubs(:path).returns('/foo') + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) + end + + it "should pass HTTP request parameters to model destroy" do + setup_destroy_request + @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) + @mock_model_class.expects(:destroy).with do |key, args| + key == 'key' and args[:foo] == :baz and args[:bar] == :xyzzy + end + @handler.service(@mock_request, @mock_response) + end + + it "should generate a 200 response when a model destroy call succeeds" do + setup_destroy_request + @mock_response.expects(:status=).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a serialized success result when a model destroy call succeeds" do + setup_destroy_request + @mock_model_class.stubs(:destroy).returns(true) + @mock_response.expects(:body=).with("--- true\n") + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by search" do + setup_search_request + @mock_model_class.expects(:search).raises(ArgumentError) + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) end - @handler.service(@mock_request, @mock_response) end - it "should pass HTTP request parameters to model save" do - setup_save_request - @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) - @mock_model_instance.expects(:save).with do |args| - args[:data] == 'This is a fake request body' and args[:foo] == :baz and args[:bar] == :xyzzy + describe "when saving a model instance" do + it "should fail to save model if data is not specified" do + @mock_request.stubs(:request_method).returns('PUT') + @mock_request.stubs(:path).returns('/foo') + @mock_request.stubs(:body).returns('') + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) + end + + it "should pass HTTP request parameters to model save" do + setup_save_request + @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) + @mock_model_instance.expects(:save).with do |args| + args[:data] == 'This is a fake request body' and args[:foo] == :baz and args[:bar] == :xyzzy + end + @handler.service(@mock_request, @mock_response) + end + + it "should generate a 200 response when a model save call succeeds" do + setup_save_request + @mock_response.expects(:status=).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a serialized object when a model save call succeeds" do + setup_save_request + @mock_model_instance.stubs(:save).returns(@mock_model_instance) + @mock_model_instance.expects(:to_yaml).returns('foo') + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by save" do + setup_save_request + @mock_model_instance.expects(:save).raises(ArgumentError) + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) end - @handler.service(@mock_request, @mock_response) - end - - it "should generate a 200 response when a model find call succeeds" do - setup_find_request - @mock_response.expects(:status=).with(200) - @handler.process(@mock_request, @mock_response) - end - - it "should generate a 200 response when a model search call succeeds" do - setup_search_request - @mock_response.expects(:status=).with(200) - @handler.process(@mock_request, @mock_response) - end - - it "should generate a 200 response when a model destroy call succeeds" do - setup_destroy_request - @mock_response.expects(:status=).with(200) - @handler.process(@mock_request, @mock_response) end - it "should generate a 200 response when a model save call succeeds" do - setup_save_request - @mock_response.expects(:status=).with(200) - @handler.process(@mock_request, @mock_response) - end + describe "when searching for model instances" do + it "should pass HTTP request parameters to model search" do + setup_search_request + @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) + @mock_model_class.expects(:search).with do |args| + args[:foo] == :baz and args[:bar] == :xyzzy + end.returns([]) + @handler.service(@mock_request, @mock_response) + end - it "should return a serialized object when a model find call succeeds" do - setup_find_request - @mock_model_instance = stub('model instance') - @mock_model_instance.expects(:to_yaml) - @mock_model_class.stubs(:find).returns(@mock_model_instance) - @handler.process(@mock_request, @mock_response) - end - - it "should return a list of serialized objects when a model search call succeeds" do - setup_search_request - mock_matches = [1..5].collect {|i| mock("model instance #{i}", :to_yaml => "model instance #{i}") } - @mock_model_class.stubs(:search).returns(mock_matches) - @handler.process(@mock_request, @mock_response) - end - - it "should return a serialized success result when a model destroy call succeeds" do - setup_destroy_request - @mock_model_class.stubs(:destroy).returns(true) - @mock_response.expects(:body=).with("--- true\n") - @handler.process(@mock_request, @mock_response) - end - - it "should return a serialized object when a model save call succeeds" do - setup_save_request - @mock_model_instance.stubs(:save).returns(@mock_model_instance) - @mock_model_instance.expects(:to_yaml).returns('foo') - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by find" do - setup_find_request - @mock_model_class.expects(:find).raises(ArgumentError) - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) + it "should generate a 200 response when a model search call succeeds" do + setup_search_request + @mock_response.expects(:status=).with(200) + @handler.process(@mock_request, @mock_response) + end + + it "should return a list of serialized objects when a model search call succeeds" do + setup_search_request + mock_matches = [1..5].collect {|i| mock("model instance #{i}", :to_yaml => "model instance #{i}") } + @mock_model_class.stubs(:search).returns(mock_matches) + @handler.process(@mock_request, @mock_response) + end + + it "should serialize a controller exception when an exception is thrown by destroy" do + setup_destroy_request + @mock_model_class.expects(:destroy).raises(ArgumentError) + @mock_response.expects(:status=).with(404) + @handler.process(@mock_request, @mock_response) + end end - it "should serialize a controller exception when an exception is thrown by search" do - setup_search_request - @mock_model_class.expects(:search).raises(ArgumentError) - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by destroy" do - setup_destroy_request - @mock_model_class.expects(:destroy).raises(ArgumentError) - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - - it "should serialize a controller exception when an exception is thrown by save" do - setup_save_request - @mock_model_instance.expects(:save).raises(ArgumentError) - @mock_response.expects(:status=).with(404) - @handler.process(@mock_request, @mock_response) - end - it "should serialize a controller exception if the request fails" do setup_bad_request @mock_response.expects(:status=).with(404) -- cgit From dab9deb312e7d66048060d079253117b0ee2102d Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 1 Apr 2008 23:01:42 -0500 Subject: bringing Indirector::REST specs to mongrel-land as well. --- spec/integration/indirector/rest.rb | 156 ++++++++++++++++++++++++++---------- 1 file changed, 112 insertions(+), 44 deletions(-) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 9ac2bc997..b92ff30bf 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -25,67 +25,135 @@ end describe Puppet::Indirector::REST do - before :each do - Puppet[:servertype] = 'webrick' - @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :test_indirected_foo ] } - @server = Puppet::Network::Server.new(@params) - @server.listen + describe "when using webrick" do + before :each do + Puppet[:servertype] = 'webrick' + @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :test_indirected_foo ] } + @server = Puppet::Network::Server.new(@params) + @server.listen - # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. - Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) - Puppet::TestIndirectedFoo.terminus_class = :rest - end + # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. + Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) + Puppet::TestIndirectedFoo.terminus_class = :rest + end - describe "when finding a model instance over REST" do - describe "when a matching model instance can be found" do - before :each do - @model_instance = Puppet::TestIndirectedFoo.new(23) - @mock_model = stub('faked model', :find => @model_instance) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) - end + describe "when finding a model instance over REST" do + describe "when a matching model instance can be found" do + before :each do + @model_instance = Puppet::TestIndirectedFoo.new(23) + @mock_model = stub('faked model', :find => @model_instance) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end - it "should not fail" do - lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error - end + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error + end - it 'should return an instance of the model class' do - Puppet::TestIndirectedFoo.find('bar').class.should == Puppet::TestIndirectedFoo - end + it 'should return an instance of the model class' do + Puppet::TestIndirectedFoo.find('bar').class.should == Puppet::TestIndirectedFoo + end - it 'should return the instance of the model class associated with the provided lookup key' do - Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value - end + it 'should return the instance of the model class associated with the provided lookup key' do + Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value + end - it 'should set a version timestamp on model instance' do - Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + it 'should set a version timestamp on model instance' do + Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + end end - end - describe "when no matching model instance can be found" do - before :each do - @mock_model = stub('faked model', :find => nil) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :find => nil) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return nil" do + Puppet::TestIndirectedFoo.find('bar').should be_nil + end end + + describe "when an exception is encountered in looking up a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:find).raises(RuntimeError) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end - it "should return nil" do - Puppet::TestIndirectedFoo.find('bar').should be_nil + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + end end end + + after :each do + @server.unlisten + end + end + + describe "when using mongrel" do + before :each do + Puppet[:servertype] = 'mongrel' + @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :test_indirected_foo ] } + @server = Puppet::Network::Server.new(@params) + @server.listen + + # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. + Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) + Puppet::TestIndirectedFoo.terminus_class = :rest + end + + describe "when finding a model instance over REST" do + describe "when a matching model instance can be found" do + before :each do + @model_instance = Puppet::TestIndirectedFoo.new(23) + @mock_model = stub('faked model', :find => @model_instance) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error + end + + it 'should return an instance of the model class' do + Puppet::TestIndirectedFoo.find('bar').class.should == Puppet::TestIndirectedFoo + end + + it 'should return the instance of the model class associated with the provided lookup key' do + Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value + end + + it 'should set a version timestamp on model instance' do + Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + end + end - describe "when an exception is encountered in looking up a model instance" do - before :each do - @mock_model = stub('faked model') - @mock_model.stubs(:find).raises(RuntimeError) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :find => nil) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return nil" do + Puppet::TestIndirectedFoo.find('bar').should be_nil + end end + + describe "when an exception is encountered in looking up a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:find).raises(RuntimeError) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end - it "should raise an exception" do - lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + end end end - end - after :each do - @server.unlisten + after :each do + @server.unlisten + end end end \ No newline at end of file -- cgit From cebb677008143894b9ebfb9b93755fc394b8bef7 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 1 Apr 2008 23:08:33 -0500 Subject: ensure that we only run the mongrel specs when mongrel is available as a feature --- spec/integration/indirector/rest.rb | 2 ++ spec/integration/network/server/mongrel.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index b92ff30bf..1025a7f1a 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -92,6 +92,8 @@ describe Puppet::Indirector::REST do end describe "when using mongrel" do + confine "Mongrel is not available" => Puppet.features.mongrel? + before :each do Puppet[:servertype] = 'mongrel' @params = { :address => "127.0.0.1", :port => 34343, :handlers => [ :test_indirected_foo ] } diff --git a/spec/integration/network/server/mongrel.rb b/spec/integration/network/server/mongrel.rb index 7bf8291a3..65caf78c9 100644 --- a/spec/integration/network/server/mongrel.rb +++ b/spec/integration/network/server/mongrel.rb @@ -4,6 +4,8 @@ require 'socket' describe Puppet::Network::Server do describe "when using mongrel" do + confine "Mongrel is not available" => Puppet.features.mongrel? + before :each do Puppet[:servertype] = 'mongrel' @params = { :address => "127.0.0.1", :port => 34346, :handlers => [ :node ] } -- cgit From a7f2dd464b46bfd55470a6bf679235552d200216 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 1 Apr 2008 23:39:59 -0500 Subject: placeholders for integration specs on final REST methods --- spec/integration/indirector/rest.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 1025a7f1a..f8084e39a 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -86,6 +86,18 @@ describe Puppet::Indirector::REST do end end + describe "when saving a model instance over REST" do + it "needs more specs" + end + + describe "when searching for model instances over REST" do + it "needs more specs" + end + + describe "when destroying a model instance over REST" do + it "needs more specs" + end + after :each do @server.unlisten end @@ -154,6 +166,18 @@ describe Puppet::Indirector::REST do end end + describe "when saving a model instance over REST" do + it "needs more specs" + end + + describe "when searching for model instances over REST" do + it "needs more specs" + end + + describe "when destroying a model instance over REST" do + it "needs more specs" + end + after :each do @server.unlisten end -- cgit From b75048202219f2e07a211df3400a0ee88ccfd208 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 1 Apr 2008 23:40:28 -0500 Subject: unit specs and implementation for Indirector::REST#search method --- lib/puppet/indirector/rest.rb | 8 ++++++ spec/unit/indirector/rest.rb | 60 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 4c54183c6..690c79632 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -13,4 +13,12 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus raise YAML.load(network_result) if network_result =~ %r{--- !ruby/exception} decoded_result = indirection.model.from_yaml(network_result) end + + def search(key, options = {}) + network_results = network_fetch("#{indirection.name}s/#{key}") + raise YAML.load(network_results) if network_results =~ %r{--- !ruby/exception} + decoded_results = network_results.collect do |result| + indirection.model.from_yaml(result) + end + end end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 56db8f509..791ef862a 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -6,8 +6,9 @@ require 'puppet/indirector/rest' describe Puppet::Indirector::REST do before do Puppet::Indirector::Terminus.stubs(:register_terminus_class) - @model = mock 'model' - @indirection = stub 'indirection', :name => :mystuff, :register_terminus_type => nil, :model => @model + @model = stub('model') + @instance = stub('model instance') + @indirection = stub('indirection', :name => :mystuff, :register_terminus_type => nil, :model => @model) Puppet::Indirector::Indirection.stubs(:instance).returns(@indirection) @rest_class = Class.new(Puppet::Indirector::REST) do @@ -21,12 +22,23 @@ describe Puppet::Indirector::REST do describe "when doing a find" do before :each do - @searcher.stubs(:network_fetch).returns({:foo => 'bar'}.to_yaml) # neuter the network connection + @result = { :foo => 'bar'}.to_yaml + @searcher.stubs(:network_fetch).returns(@result) # neuter the network connection @model.stubs(:from_yaml).returns(@instance) end it "should look up the model instance over the network" do - @searcher.expects(:network_fetch).returns({:foo => 'bar'}.to_yaml) + @searcher.expects(:network_fetch).returns(@result) + @searcher.find('foo') + end + + it "should look up the model instance using the named indirection" do + @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result) + @searcher.find('foo') + end + + it "should look up the model instance using the provided key" do + @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result) @searcher.find('foo') end @@ -40,7 +52,7 @@ describe Puppet::Indirector::REST do end it "should return nil when deserialized model instance is nil" do - @model.stubs(:from_yaml).returns(@instance) + @model.stubs(:from_yaml).returns(nil) @searcher.find('foo').should be_nil end @@ -56,9 +68,41 @@ describe Puppet::Indirector::REST do end describe "when doing a search" do - it "should deserialize result data into a list of Model instances" - it "should generate an error when result data deserializes improperly" - it "should generate an error when result data specifies an error" + before :each do + @result = [1, 2] + @searcher.stubs(:network_fetch).returns(@result) + @model.stubs(:from_yaml).returns(@instance) + end + + it "should look up the model data over the network" do + @searcher.expects(:network_fetch).returns(@result) + @searcher.search('foo') + end + + it "should look up the model instance using the named indirection" do + @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}s/} }.returns(@result) + @searcher.search('foo') + end + + it "should look up the model instance using the provided key" do + @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result) + @searcher.search('foo') + end + + it "should deserialize result data into a list of Model instances" do + @model.expects(:from_yaml).at_least(2) + @searcher.search('foo') + end + + it "should generate an error when result data deserializes improperly" do + @model.stubs(:from_yaml).raises(ArgumentError) + lambda { @searcher.search('foo') }.should raise_error(ArgumentError) + end + + it "should generate an error when result data specifies an error" do + @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml) + lambda { @searcher.search('foo') }.should raise_error(RuntimeError) + end end describe "when doing a save" do -- cgit From e8caf135a4a378d54bf62f8c37064ee3ccc508e9 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 00:06:30 -0500 Subject: making search work over REST, w/ unit & integration specs --- lib/puppet/indirector/rest.rb | 4 +-- lib/puppet/network/http/handler.rb | 2 +- spec/integration/indirector/rest.rb | 63 ++++++++++++++++++++++++++++++++++--- spec/unit/indirector/rest.rb | 2 +- 4 files changed, 62 insertions(+), 9 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 690c79632..0c86b2706 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -17,8 +17,6 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus def search(key, options = {}) network_results = network_fetch("#{indirection.name}s/#{key}") raise YAML.load(network_results) if network_results =~ %r{--- !ruby/exception} - decoded_results = network_results.collect do |result| - indirection.model.from_yaml(result) - end + decoded_results = YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } end end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index f226ae133..9e6c28512 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -32,7 +32,7 @@ module Puppet::Network::HTTP::Handler def do_search(request, response) args = params(request) - result = model.search(args).collect {|obj| obj.to_yaml } + result = model.search(args).collect {|result| result.to_yaml }.to_yaml encode_result(request, response, result) end diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index f8084e39a..b9e9513c4 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -166,15 +166,70 @@ describe Puppet::Indirector::REST do end end - describe "when saving a model instance over REST" do - it "needs more specs" + describe "when searching for model instances over REST" do + describe "when matching model instances can be found" do + before :each do + @model_instances = [ Puppet::TestIndirectedFoo.new(23), Puppet::TestIndirectedFoo.new(24) ] + @mock_model = stub('faked model', :search => @model_instances) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.search('bar') }.should_not raise_error + end + + it 'should return all matching results' do + Puppet::TestIndirectedFoo.search('bar').length.should == @model_instances.length + end + + it 'should return model instances' do + Puppet::TestIndirectedFoo.search('bar').each do |result| + result.class.should == Puppet::TestIndirectedFoo + end + end + + it 'should return the instance of the model class associated with the provided lookup key' do + Puppet::TestIndirectedFoo.search('bar').collect(&:value).should == @model_instances.collect(&:value) + end + + it 'should set a version timestamp on model instances' do + pending("Luke looking at why this version magic might not be working") do + Puppet::TestIndirectedFoo.search('bar').each do |result| + result.version.should_not be_nil + end + end + end + end + + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :find => nil) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return nil" do + Puppet::TestIndirectedFoo.find('bar').should be_nil + end + end + + describe "when an exception is encountered in looking up a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:find).raises(RuntimeError) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + end + end end - describe "when searching for model instances over REST" do + describe "when destroying a model instance over REST" do it "needs more specs" end - describe "when destroying a model instance over REST" do + describe "when saving a model instance over REST" do it "needs more specs" end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 791ef862a..6f4285765 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -69,7 +69,7 @@ describe Puppet::Indirector::REST do describe "when doing a search" do before :each do - @result = [1, 2] + @result = [1, 2].to_yaml @searcher.stubs(:network_fetch).returns(@result) @model.stubs(:from_yaml).returns(@instance) end -- cgit From 07974401178a8b46314971bc7a9858cff1d9797b Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 00:12:27 -0500 Subject: updating search integration specs to include webrick --- spec/integration/indirector/rest.rb | 63 ++++++++++++++++++++++++++++++++++--- spec/unit/indirector/rest.rb | 8 ++--- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index b9e9513c4..418575fb6 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -86,15 +86,70 @@ describe Puppet::Indirector::REST do end end - describe "when saving a model instance over REST" do - it "needs more specs" + describe "when searching for model instances over REST" do + describe "when matching model instances can be found" do + before :each do + @model_instances = [ Puppet::TestIndirectedFoo.new(23), Puppet::TestIndirectedFoo.new(24) ] + @mock_model = stub('faked model', :search => @model_instances) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.search('bar') }.should_not raise_error + end + + it 'should return all matching results' do + Puppet::TestIndirectedFoo.search('bar').length.should == @model_instances.length + end + + it 'should return model instances' do + Puppet::TestIndirectedFoo.search('bar').each do |result| + result.class.should == Puppet::TestIndirectedFoo + end + end + + it 'should return the instance of the model class associated with the provided lookup key' do + Puppet::TestIndirectedFoo.search('bar').collect(&:value).should == @model_instances.collect(&:value) + end + + it 'should set a version timestamp on model instances' do + pending("Luke looking at why this version magic might not be working") do + Puppet::TestIndirectedFoo.search('bar').each do |result| + result.version.should_not be_nil + end + end + end + end + + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :find => nil) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return nil" do + Puppet::TestIndirectedFoo.find('bar').should be_nil + end + end + + describe "when an exception is encountered in looking up a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:find).raises(RuntimeError) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.find('bar') }.should raise_error(RuntimeError) + end + end end - describe "when searching for model instances over REST" do + describe "when destroying a model instance over REST" do it "needs more specs" end - describe "when destroying a model instance over REST" do + describe "when saving a model instance over REST" do it "needs more specs" end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 6f4285765..37c0480ed 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -103,15 +103,15 @@ describe Puppet::Indirector::REST do @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml) lambda { @searcher.search('foo') }.should raise_error(RuntimeError) end - end + end - describe "when doing a save" do + describe "when doing a destroy" do it "should deserialize result data into a boolean" it "should generate an error when result data deserializes improperly" it "should generate an error when result data specifies an error" end - - describe "when doing a destroy" do + + describe "when doing a save" do it "should deserialize result data into a boolean" it "should generate an error when result data deserializes improperly" it "should generate an error when result data specifies an error" -- cgit From f28f20b675737741a98b1b87b4791f736a996d40 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 00:48:54 -0500 Subject: Added support for destroy/DELETE over REST (including units & integrations on both webrick & mongrel). Added pending specs for the trivialities in the REST network_fetch and network_delete methods. Refactored YAML exception detection out into a private helper method. --- lib/puppet/indirector/rest.rb | 35 +++++++++++++---- spec/integration/indirector/rest.rb | 76 ++++++++++++++++++++++++++++++++++++- spec/unit/indirector/rest.rb | 59 ++++++++++++++++++++++++++-- 3 files changed, 157 insertions(+), 13 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 0c86b2706..076e96356 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -4,19 +4,38 @@ require 'uri' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus def network_fetch(path) - # TODO: url_encode path, set proper server + port Net::HTTP.get(URI.parse("http://127.0.0.1:34343/#{path}")) end + def network_delete(path) + Net::HTTP.start("127.0.0.1", 34343) {|x| x.delete("/#{path}").body } # weird-ass net/http library + end + def find(name, options = {}) - network_result = network_fetch("#{indirection.name}/#{name}") - raise YAML.load(network_result) if network_result =~ %r{--- !ruby/exception} - decoded_result = indirection.model.from_yaml(network_result) + network_result = network_fetch("#{indirection.name}/#{name}") + raise YAML.load(network_result) if exception?(network_result) + decoded_result = indirection.model.from_yaml(network_result) + end + + def search(name, options = {}) + network_results = network_fetch("#{indirection.name}s/#{name}") + raise YAML.load(network_results) if exception?(network_results) + decoded_results = YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } + end + + def destroy(name, options = {}) + network_result = network_delete("#{indirection.name}/#{name}") + raise YAML.load(network_result) if exception?(network_result) + decoded_result = YAML.load(network_result.to_s) + end + + def save + end - def search(key, options = {}) - network_results = network_fetch("#{indirection.name}s/#{key}") - raise YAML.load(network_results) if network_results =~ %r{--- !ruby/exception} - decoded_results = YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } + private + + def exception?(yaml_string) + yaml_string =~ %r{--- !ruby/exception} end end diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 418575fb6..46a26ee91 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -146,7 +146,43 @@ describe Puppet::Indirector::REST do end describe "when destroying a model instance over REST" do - it "needs more specs" + describe "when a matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :destroy => true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.destroy('bar') }.should_not raise_error + end + + it 'should return success' do + Puppet::TestIndirectedFoo.destroy('bar').should == true + end + end + + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :destroy => false) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return failure" do + Puppet::TestIndirectedFoo.destroy('bar').should == false + end + end + + describe "when an exception is encountered in destroying a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:destroy).raises(RuntimeError) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.destroy('bar') }.should raise_error(RuntimeError) + end + end end describe "when saving a model instance over REST" do @@ -281,7 +317,43 @@ describe Puppet::Indirector::REST do end describe "when destroying a model instance over REST" do - it "needs more specs" + describe "when a matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :destroy => true) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should not fail" do + lambda { Puppet::TestIndirectedFoo.destroy('bar') }.should_not raise_error + end + + it 'should return success' do + Puppet::TestIndirectedFoo.destroy('bar').should == true + end + end + + describe "when no matching model instance can be found" do + before :each do + @mock_model = stub('faked model', :destroy => false) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should return failure" do + Puppet::TestIndirectedFoo.destroy('bar').should == false + end + end + + describe "when an exception is encountered in destroying a model instance" do + before :each do + @mock_model = stub('faked model') + @mock_model.stubs(:destroy).raises(RuntimeError) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + end + + it "should raise an exception" do + lambda { Puppet::TestIndirectedFoo.destroy('bar') }.should raise_error(RuntimeError) + end + end end describe "when saving a model instance over REST" do diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 37c0480ed..fb3790e0c 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -19,6 +19,28 @@ describe Puppet::Indirector::REST do @searcher = @rest_class.new end + + describe "when doing a network fetch" do + it "should escape the provided path" + it "should look up the appropriate remote server" + it "should look up the appropriate remote port" + it "should use the GET http method" + it "should use the appropriate remote server" + it "should use the appropriate remote port" + it "should use the escaped provided path" + it "should return the results of the GET request" + end + + describe "when doing a network delete" do + it "should escape the provided path" + it "should look up the appropriate remote server" + it "should look up the appropriate remote port" + it "should use the delete http method" + it "should use the appropriate remote server" + it "should use the appropriate remote port" + it "should use the escaped provided path" + it "should return the results of the DELETE request" + end describe "when doing a find" do before :each do @@ -106,9 +128,40 @@ describe Puppet::Indirector::REST do end describe "when doing a destroy" do - it "should deserialize result data into a boolean" - it "should generate an error when result data deserializes improperly" - it "should generate an error when result data specifies an error" + before :each do + @result = true.to_yaml + @searcher.stubs(:network_delete).returns(@result) # neuter the network connection + @model.stubs(:from_yaml).returns(@instance) + end + + it "should look up the model instance over the network" do + @searcher.expects(:network_delete).returns(@result) + @searcher.destroy('foo') + end + + it "should look up the model instance using the named indirection" do + @searcher.expects(:network_delete).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result) + @searcher.destroy('foo') + end + + it "should look up the model instance using the provided key" do + @searcher.expects(:network_delete).with {|path| path =~ %r{/foo$} }.returns(@result) + @searcher.destroy('foo') + end + + it "should deserialize result data" do + YAML.expects(:load).with(@result) + @searcher.destroy('foo') + end + + it "should return deserialized result data" do + @searcher.destroy('foo').should == true + end + + it "should generate an error when result data specifies an error" do + @searcher.stubs(:network_delete).returns(RuntimeError.new("bogus").to_yaml) + lambda { @searcher.destroy('foo') }.should raise_error(RuntimeError) + end end describe "when doing a save" do -- cgit From 1befd1d8f1d389f425a36b1515dbba7408ac6238 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 01:15:53 -0500 Subject: work-in-progress; playing with refactoring network_* methods inside Indirector::REST --- lib/puppet/indirector/rest.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 076e96356..33ae34006 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -4,11 +4,20 @@ require 'uri' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus def network_fetch(path) - Net::HTTP.get(URI.parse("http://127.0.0.1:34343/#{path}")) + network(path, 'get') end def network_delete(path) - Net::HTTP.start("127.0.0.1", 34343) {|x| x.delete("/#{path}").body } # weird-ass net/http library + network(path, 'delete') + end + + def network_put(path, data) + network(path, 'put', data) + end + + def network(path, meth, data = nil) + # TODO: include data here, for #save + Net::HTTP.start("127.0.0.1", 34343) {|x| x.send(meth.to_sym, "/#{path}").body } # weird-ass net/http library end def find(name, options = {}) -- cgit From 99b295b8301d7a89c97ecdc1d636c2d2b7f1ae8e Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 19:28:11 -0500 Subject: disabling caching for Puppet::Indirector::Indirection as it was causing hella problems with testing save without caching; judging my luke's blog this is going to be rewritten somehow anyway --- lib/puppet/indirector/indirection.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 15358a801..606234dd0 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -242,7 +242,6 @@ class Puppet::Indirector::Indirection 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.each do |instance| instance.expiration ||= self.expiration end -- cgit From 93bc1a946f2da6e7c78a38ff90dac8a20b1bcbc7 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 19:29:04 -0500 Subject: adding REST save support, with integration tests. A handful of unit tests in that area now need to be updated. --- lib/puppet/indirector/rest.rb | 27 ++++++----- lib/puppet/network/http/handler.rb | 12 +++-- spec/integration/indirector/rest.rb | 93 ++++++++++++++++++++++++++++++++++++- spec/unit/network/server.rb | 2 - 4 files changed, 112 insertions(+), 22 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 33ae34006..889f63648 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -4,47 +4,46 @@ require 'uri' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus def network_fetch(path) - network(path, 'get') + Net::HTTP.start("127.0.0.1", 34343) {|x| x.get("/#{path}").body } end def network_delete(path) - network(path, 'delete') + Net::HTTP.start("127.0.0.1", 34343) {|x| x.delete("/#{path}").body } end def network_put(path, data) - network(path, 'put', data) - end - - def network(path, meth, data = nil) - # TODO: include data here, for #save - Net::HTTP.start("127.0.0.1", 34343) {|x| x.send(meth.to_sym, "/#{path}").body } # weird-ass net/http library + Net::HTTP.start("127.0.0.1", 34343) {|x| x.put("/#{path}", data).body } end def find(name, options = {}) network_result = network_fetch("#{indirection.name}/#{name}") raise YAML.load(network_result) if exception?(network_result) - decoded_result = indirection.model.from_yaml(network_result) + indirection.model.from_yaml(network_result) end def search(name, options = {}) network_results = network_fetch("#{indirection.name}s/#{name}") raise YAML.load(network_results) if exception?(network_results) - decoded_results = YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } + YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } end def destroy(name, options = {}) network_result = network_delete("#{indirection.name}/#{name}") raise YAML.load(network_result) if exception?(network_result) - decoded_result = YAML.load(network_result.to_s) + YAML.load(network_result.to_s) end - def save - + def save(obj, options = {}) + network_result = network_put("#{indirection.name}/", obj.to_yaml) + # TODO: swap these two lines out: + raise network_result.inspect if exception?(network_result) + # raise YAML.load(network_result) if exception?(network_result) + indirection.model.from_yaml(network_result) end private def exception?(yaml_string) - yaml_string =~ %r{--- !ruby/exception} + yaml_string =~ %r{--- !ruby/exception} end end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 9e6c28512..7113c92d3 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -44,13 +44,17 @@ module Puppet::Network::HTTP::Handler end def do_save(request, response) - data = body(request) + data = body(request).to_s raise ArgumentError, "No data to save" if !data or data.empty? - args = params(request) - obj = model.new - result = obj.save(args.merge(:data => data)).to_yaml + # args = params(request) + obj = model.from_yaml(data) + result = save_object(obj).to_yaml encode_result(request, response, result) end + + def save_object(obj) + obj.save + end def do_exception(request, response, exception, status=404) encode_result(request, response, exception.to_yaml, status) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 46a26ee91..a969a975b 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -9,6 +9,7 @@ class Puppet::TestIndirectedFoo indirects :test_indirected_foo, :terminus_setting => :test_indirected_foo_terminus attr_reader :value + attr_accessor :version def initialize(value = 0) @value = value @@ -17,6 +18,10 @@ class Puppet::TestIndirectedFoo def self.from_yaml(yaml) YAML.load(yaml) end + + def name + "bob" + end end # empty Terminus class -- this would normally have to be in a directory findable by the autoloader, but we short-circuit that below @@ -186,7 +191,49 @@ describe Puppet::Indirector::REST do end describe "when saving a model instance over REST" do - it "needs more specs" + before :each do + @instance = Puppet::TestIndirectedFoo.new(42) + @mock_model = stub('faked model', :from_yaml => @instance) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:save_object).returns(@instance) + end + + describe "when a successful save can be performed" do + before :each do + end + + it "should not fail" do + lambda { @instance.save }.should_not raise_error + end + + it 'should return an instance of the model class' do + @instance.save.class.should == Puppet::TestIndirectedFoo + end + + it 'should return a matching instance of the model class' do + @instance.save.value.should == @instance.value + end + end + + describe "when a save cannot be completed" do + before :each do + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:save_object).returns(false) + end + + it "should return failure" do + @instance.save.should == false + end + end + + describe "when an exception is encountered in performing a save" do + before :each do + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:save_object).raises(RuntimeError) + end + + it "should raise an exception" do + lambda { @instance.save }.should raise_error(RuntimeError) + end + end end after :each do @@ -357,7 +404,49 @@ describe Puppet::Indirector::REST do end describe "when saving a model instance over REST" do - it "needs more specs" + before :each do + @instance = Puppet::TestIndirectedFoo.new(42) + @mock_model = stub('faked model', :from_yaml => @instance) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:save_object).returns(@instance) + end + + describe "when a successful save can be performed" do + before :each do + end + + it "should not fail" do + lambda { @instance.save }.should_not raise_error + end + + it 'should return an instance of the model class' do + @instance.save.class.should == Puppet::TestIndirectedFoo + end + + it 'should return a matching instance of the model class' do + @instance.save.value.should == @instance.value + end + end + + describe "when a save cannot be completed" do + before :each do + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:save_object).returns(false) + end + + it "should return failure" do + @instance.save.should == false + end + end + + describe "when an exception is encountered in performing a save" do + before :each do + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:save_object).raises(RuntimeError) + end + + it "should raise an exception" do + lambda { @instance.save }.should raise_error(RuntimeError) + end + end end after :each do diff --git a/spec/unit/network/server.rb b/spec/unit/network/server.rb index 2e02ffbc3..4e47c22fd 100644 --- a/spec/unit/network/server.rb +++ b/spec/unit/network/server.rb @@ -309,8 +309,6 @@ end describe Class.new, "put these somewhere" do - it "should allow indirections to deny access to services based upon which client is connecting, or whether the client is authorized" - it "should deny access to clients based upon rules" it "should have the ability to use a class-level from_ hook (from_yaml, from_text, etc.) that can be called, based on content-type header, to allow for different deserializations of an object" it "should allow from_* on the inbound :data packet (look at its content_type) when doing a PUT/.new.save" it "should prepend a rest version number on the path (w00t)" -- cgit From 9187a347306f547805c175b1f288a97da721cc11 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 20:28:57 -0500 Subject: updating mongrel/webrick unit tests to match integration-tested version of REST save functionality --- spec/unit/network/http/mongrel/rest.rb | 12 ++---------- spec/unit/network/http/webrick/rest.rb | 14 ++------------ 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb index 3de314852..b483bbd46 100644 --- a/spec/unit/network/http/mongrel/rest.rb +++ b/spec/unit/network/http/mongrel/rest.rb @@ -73,7 +73,7 @@ describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do 'QUERY_STRING' => '' }.merge(params)) @mock_request.stubs(:body).returns('this is a fake request body') @mock_model_instance = stub('indirected model instance', :save => true) - @mock_model_class.stubs(:new).returns(@mock_model_instance) + @mock_model_class.stubs(:from_yaml).returns(@mock_model_instance) end def setup_bad_request @@ -100,7 +100,7 @@ describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do it "should call the model save method if the request represents an HTTP PUT" do setup_save_request - @mock_model_instance.expects(:save).with(:data => 'this is a fake request body') + @mock_model_instance.expects(:save) @handler.process(@mock_request, @mock_response) end @@ -210,14 +210,6 @@ describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do @handler.process(@mock_request, @mock_response) end - it "should pass HTTP request parameters to model save" do - setup_save_request('QUERY_STRING' => 'foo=baz&bar=xyzzy') - @mock_model_instance.expects(:save).with do |args| - args[:data] == 'this is a fake request body' and args['foo'] == 'baz' and args['bar'] == 'xyzzy' - end - @handler.process(@mock_request, @mock_response) - end - it "should generate a 200 response when a model save call succeeds" do setup_save_request @mock_response.expects(:start).with(200) diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb index cdc1164fc..b7bd33880 100644 --- a/spec/unit/network/http/webrick/rest.rb +++ b/spec/unit/network/http/webrick/rest.rb @@ -64,7 +64,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @mock_request.stubs(:path).returns('/foo') @mock_request.stubs(:body).returns('This is a fake request body') @mock_model_instance = stub('indirected model instance', :save => true) - @mock_model_class.stubs(:new).returns(@mock_model_instance) + @mock_model_class.stubs(:from_yaml).returns(@mock_model_instance) end def setup_bad_request @@ -93,8 +93,7 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do it "should call the model save method if the request represents an HTTP PUT" do setup_save_request - @mock_model_instance.expects(:save).with(:data => 'This is a fake request body') - @mock_model_class.expects(:new).returns(@mock_model_instance) + @mock_model_instance.expects(:save) @handler.service(@mock_request, @mock_response) end @@ -212,15 +211,6 @@ describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do @handler.process(@mock_request, @mock_response) end - it "should pass HTTP request parameters to model save" do - setup_save_request - @mock_request.stubs(:query).returns(:foo => :baz, :bar => :xyzzy) - @mock_model_instance.expects(:save).with do |args| - args[:data] == 'This is a fake request body' and args[:foo] == :baz and args[:bar] == :xyzzy - end - @handler.service(@mock_request, @mock_response) - end - it "should generate a 200 response when a model save call succeeds" do setup_save_request @mock_response.expects(:status=).with(200) -- cgit From 75bf05d516ebdace19703c1c1bca5d2cc65d1001 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 20:30:20 -0500 Subject: removed a debugging helper from the Indirector::Rest#save method --- lib/puppet/indirector/rest.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 889f63648..546cbdaf7 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -35,9 +35,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus def save(obj, options = {}) network_result = network_put("#{indirection.name}/", obj.to_yaml) - # TODO: swap these two lines out: - raise network_result.inspect if exception?(network_result) - # raise YAML.load(network_result) if exception?(network_result) + raise YAML.load(network_result) if exception?(network_result) indirection.model.from_yaml(network_result) end -- cgit From aed13754459e11bcc04a0b14228f5f53b4cd4c4c Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 20:41:16 -0500 Subject: make sure unit indirector specs are working with #save; fill out network_put pending specs --- spec/unit/indirector/rest.rb | 56 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index fb3790e0c..38ab0d634 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -41,6 +41,17 @@ describe Puppet::Indirector::REST do it "should use the escaped provided path" it "should return the results of the DELETE request" end + + describe "when doing a network put" do + it "should escape the provided path" + it "should look up the appropriate remote server" + it "should look up the appropriate remote port" + it "should use the delete http method" + it "should use the appropriate remote server" + it "should use the appropriate remote port" + it "should use the escaped provided path" + it "should return the results of the DELETE request" + end describe "when doing a find" do before :each do @@ -165,8 +176,47 @@ describe Puppet::Indirector::REST do end describe "when doing a save" do - it "should deserialize result data into a boolean" - it "should generate an error when result data deserializes improperly" - it "should generate an error when result data specifies an error" + before :each do + @result = { :foo => 'bar'}.to_yaml + @searcher.stubs(:network_put).returns(@result) # neuter the network connection + @model.stubs(:from_yaml).returns(@instance) + end + + it "should save the model instance over the network" do + @searcher.expects(:network_put).returns(@result) + @searcher.save(@instance) + end + + it "should save the model instance using the named indirection" do + @searcher.expects(:network_put).with do |path, data| + path =~ %r{^#{@indirection.name.to_s}/} and + data == @instance.to_yaml + end.returns(@result) + @searcher.save(@instance) + end + + it "should deserialize result data to a Model instance" do + @model.expects(:from_yaml) + @searcher.save(@instance) + end + + it "should return the resulting deserialized Model instance" do + @searcher.save(@instance).should == @instance + end + + it "should return nil when deserialized model instance is nil" do + @model.stubs(:from_yaml).returns(nil) + @searcher.save(@instance).should be_nil + end + + it "should generate an error when result data deserializes improperly" do + @model.stubs(:from_yaml).raises(ArgumentError) + lambda { @searcher.save(@instance) }.should raise_error(ArgumentError) + end + + it "should generate an error when result data specifies an error" do + @searcher.stubs(:network_put).returns(RuntimeError.new("bogus").to_yaml) + lambda { @searcher.save(@instance) }.should raise_error(RuntimeError) + end end end -- cgit From a0804ae29a4d2be7b3f15015f87f5b274e95c7bd Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 20:45:51 -0500 Subject: adding rest_connection_details helper to Indirector::REST -- will need to be overridden to lookup the real connection details --- lib/puppet/indirector/rest.rb | 5 +++++ spec/unit/indirector/rest.rb | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 546cbdaf7..e6a7bd6cf 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -3,6 +3,11 @@ require 'uri' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus + + def rest_connection_details + { :host => '127.0.0.1', :port => 34343 } + end + def network_fetch(path) Net::HTTP.start("127.0.0.1", 34343) {|x| x.get("/#{path}").body } end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 38ab0d634..dabb928cf 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -20,6 +20,18 @@ describe Puppet::Indirector::REST do @searcher = @rest_class.new end + describe "when locating the rest connection" do + it 'should look somewhere meaningful for connection details' + + it "should return a host" do + @searcher.rest_connection_details[:host].should == '127.0.0.1' + end + + it "should return a port" do + @searcher.rest_connection_details[:port].should == 34343 + end + end + describe "when doing a network fetch" do it "should escape the provided path" it "should look up the appropriate remote server" -- cgit From 04aba5253d774fae013919605363022781f16d55 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Wed, 2 Apr 2008 21:27:14 -0500 Subject: fill out specs for network_* methods; refactor lowest-level network hooks --- lib/puppet/indirector/rest.rb | 12 ++- spec/unit/indirector/rest.rb | 184 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 166 insertions(+), 30 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index e6a7bd6cf..34596cad4 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -5,19 +5,19 @@ require 'uri' class Puppet::Indirector::REST < Puppet::Indirector::Terminus def rest_connection_details - { :host => '127.0.0.1', :port => 34343 } + { :host => '127.0.0.1', :port => 34343 } end def network_fetch(path) - Net::HTTP.start("127.0.0.1", 34343) {|x| x.get("/#{path}").body } + network {|conn| conn.get("/#{path}").body } end def network_delete(path) - Net::HTTP.start("127.0.0.1", 34343) {|x| x.delete("/#{path}").body } + network {|conn| conn.delete("/#{path}").body } end def network_put(path, data) - Net::HTTP.start("127.0.0.1", 34343) {|x| x.put("/#{path}", data).body } + network {|conn| conn.put("/#{path}", data).body } end def find(name, options = {}) @@ -46,6 +46,10 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus private + def network(&block) + Net::HTTP.start(rest_connection_details[:host], rest_connection_details[:port]) {|conn| yield(conn) } + end + def exception?(yaml_string) yaml_string =~ %r{--- !ruby/exception} end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index dabb928cf..e511098ab 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -20,8 +20,10 @@ describe Puppet::Indirector::REST do @searcher = @rest_class.new end - describe "when locating the rest connection" do - it 'should look somewhere meaningful for connection details' + describe "when locating the REST connection" do + it 'should look somewhere meaningful for connection details' do + pending("Luke enlightening us on where to find the appropriate connection details") + end it "should return a host" do @searcher.rest_connection_details[:host].should == '127.0.0.1' @@ -33,36 +35,162 @@ describe Puppet::Indirector::REST do end describe "when doing a network fetch" do - it "should escape the provided path" - it "should look up the appropriate remote server" - it "should look up the appropriate remote port" - it "should use the GET http method" - it "should use the appropriate remote server" - it "should use the appropriate remote port" - it "should use the escaped provided path" - it "should return the results of the GET request" + before :each do + Net::HTTP.stubs(:start).returns('result') + @details = { :host => '127.0.0.1', :port => 34343 } + @searcher.stubs(:rest_connection_details).returns(@details) + end + + it "should accept a path" do + lambda { @search.network_fetch('foo') }.should_not raise_error(ArgumentError) + end + + it "should require a path" do + lambda { @searcher.network_fetch }.should raise_error(ArgumentError) + end + + it "should look up connection details" do + @searcher.expects(:rest_connection_details).returns(@details) + @searcher.network_fetch('foo') + end + + it "should use the GET http method" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = mock('mock http connection', :get => @mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_fetch('foo') + end + + it "should use the appropriate remote server" do + Net::HTTP.expects(:start).with {|host, port| host == @details[:host] } + @searcher.network_fetch('foo') + end + + it "should use the appropriate remote port" do + Net::HTTP.expects(:start).with {|host, port| port == @details[:port] } + @searcher.network_fetch('foo') + end + + it "should use the provided path" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = stub('mock http connection') + @mock_connection.expects(:get).with('/foo').returns(@mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_fetch('foo') + end + + it "should return the results of the GET request" do + @searcher.network_fetch('foo').should == 'result' + end end describe "when doing a network delete" do - it "should escape the provided path" - it "should look up the appropriate remote server" - it "should look up the appropriate remote port" - it "should use the delete http method" - it "should use the appropriate remote server" - it "should use the appropriate remote port" - it "should use the escaped provided path" - it "should return the results of the DELETE request" + before :each do + Net::HTTP.stubs(:start).returns('result') + @details = { :host => '127.0.0.1', :port => 34343 } + @searcher.stubs(:rest_connection_details).returns(@details) + end + + it "should accept a path" do + lambda { @search.network_delete('foo') }.should_not raise_error(ArgumentError) + end + + it "should require a path" do + lambda { @searcher.network_delete }.should raise_error(ArgumentError) + end + + it "should look up connection details" do + @searcher.expects(:rest_connection_details).returns(@details) + @searcher.network_delete('foo') + end + + it "should use the DELETE http method" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = mock('mock http connection', :delete => @mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_delete('foo') + end + + it "should use the appropriate remote server" do + Net::HTTP.expects(:start).with {|host, port| host == @details[:host] } + @searcher.network_delete('foo') + end + + it "should use the appropriate remote port" do + Net::HTTP.expects(:start).with {|host, port| port == @details[:port] } + @searcher.network_delete('foo') + end + + it "should use the provided path" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = stub('mock http connection') + @mock_connection.expects(:delete).with('/foo').returns(@mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_delete('foo') + end + + it "should return the results of the DELETE request" do + @searcher.network_delete('foo').should == 'result' + end end describe "when doing a network put" do - it "should escape the provided path" - it "should look up the appropriate remote server" - it "should look up the appropriate remote port" - it "should use the delete http method" - it "should use the appropriate remote server" - it "should use the appropriate remote port" - it "should use the escaped provided path" - it "should return the results of the DELETE request" + before :each do + Net::HTTP.stubs(:start).returns('result') + @details = { :host => '127.0.0.1', :port => 34343 } + @data = { :foo => 'bar' } + @searcher.stubs(:rest_connection_details).returns(@details) + end + + it "should accept a path and data" do + lambda { @search.network_put('foo', @data) }.should_not raise_error(ArgumentError) + end + + it "should require a path and data" do + lambda { @searcher.network_put('foo') }.should raise_error(ArgumentError) + end + + it "should look up connection details" do + @searcher.expects(:rest_connection_details).returns(@details) + @searcher.network_put('foo', @data) + end + + it "should use the appropriate remote server" do + Net::HTTP.expects(:start).with {|host, port| host == @details[:host] } + @searcher.network_put('foo', @data) + end + + it "should use the appropriate remote port" do + Net::HTTP.expects(:start).with {|host, port| port == @details[:port] } + @searcher.network_put('foo', @data) + end + + it "should use the PUT http method" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = mock('mock http connection', :put => @mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_put('foo', @data) + end + + it "should use the provided path" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = stub('mock http connection') + @mock_connection.expects(:put).with {|path, data| path == '/foo' }.returns(@mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_put('foo', @data) + end + + it "should use the provided data" do + @mock_result = stub('mock result', :body => 'result') + @mock_connection = stub('mock http connection') + @mock_connection.expects(:put).with {|path, data| data == @data }.returns(@mock_result) + @searcher.stubs(:network).yields(@mock_connection) + @searcher.network_put('foo', @data) + end + + it "should return the results of the PUT request" do + @searcher.network_put('foo', @data).should == 'result' + end end describe "when doing a find" do @@ -194,6 +322,10 @@ describe Puppet::Indirector::REST do @model.stubs(:from_yaml).returns(@instance) end + it "should re-enable caching in the terminus" do + pending("Luke taking a look at (a) why that doesn't seem to work with this save, or (b) rewriting it, like he seems to indicate in his blog is going to happen") + end + it "should save the model instance over the network" do @searcher.expects(:network_put).returns(@result) @searcher.save(@instance) -- cgit From a6a397b21ce9306307c7614b671de63d74d8141e Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 11 Apr 2008 13:50:36 -0500 Subject: The 'destroy' method in the indirection now returns the results of destroying, so they can return true or false. --- lib/puppet/indirector/indirection.rb | 4 ++-- spec/unit/indirector/indirection.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 606234dd0..05464f8c9 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -225,14 +225,14 @@ class Puppet::Indirector::Indirection request = request(:destroy, key, *args) terminus = prepare(request) - terminus.destroy(request) + result = terminus.destroy(request) if cache? and cached = cache.find(request(:find, key, *args)) # Reuse the existing request, since it's equivalent. cache.destroy(request) end - nil + result end # Search for more than one instance. Should always return an array. diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb index e8ab9633b..cefd0557e 100755 --- a/spec/unit/indirector/indirection.rb +++ b/spec/unit/indirector/indirection.rb @@ -356,9 +356,9 @@ describe Puppet::Indirector::Indirection do it_should_behave_like "Indirection Delegator" it_should_behave_like "Delegation Authorizer" - it "should return nil" do - @terminus.stubs(:destroy) - @indirection.destroy("/my/key").should be_nil + it "should return the result of removing the instance" do + @terminus.stubs(:destroy).returns "yayness" + @indirection.destroy("/my/key").should == "yayness" end describe "when caching is enabled" do -- cgit From cb617f20ed6e0af362937760f33f5ddc34e626ee Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 11 Apr 2008 13:53:27 -0500 Subject: Making the changes necessary to get the REST support to work with the current state of the indirection work, including using a request object and an expiration date. --- lib/puppet/indirector/rest.rb | 16 +++++----- spec/integration/indirector/rest.rb | 20 ++++++------ spec/unit/indirector/rest.rb | 62 +++++++++++++++++++++---------------- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 34596cad4..26f7736f3 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -20,26 +20,26 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus network {|conn| conn.put("/#{path}", data).body } end - def find(name, options = {}) - network_result = network_fetch("#{indirection.name}/#{name}") + def find(request) + network_result = network_fetch("#{indirection.name}/#{request.key}") raise YAML.load(network_result) if exception?(network_result) indirection.model.from_yaml(network_result) end - def search(name, options = {}) - network_results = network_fetch("#{indirection.name}s/#{name}") + def search(request) + network_results = network_fetch("#{indirection.name}s/#{request.key}") raise YAML.load(network_results) if exception?(network_results) YAML.load(network_results.to_s).collect {|result| indirection.model.from_yaml(result) } end - def destroy(name, options = {}) - network_result = network_delete("#{indirection.name}/#{name}") + def destroy(request) + network_result = network_delete("#{indirection.name}/#{request.key}") raise YAML.load(network_result) if exception?(network_result) YAML.load(network_result.to_s) end - def save(obj, options = {}) - network_result = network_put("#{indirection.name}/", obj.to_yaml) + def save(request) + network_result = network_put("#{indirection.name}/", request.instance.to_yaml) raise YAML.load(network_result) if exception?(network_result) indirection.model.from_yaml(network_result) end diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index a969a975b..8e3c92d5c 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -9,7 +9,6 @@ class Puppet::TestIndirectedFoo indirects :test_indirected_foo, :terminus_setting => :test_indirected_foo_terminus attr_reader :value - attr_accessor :version def initialize(value = 0) @value = value @@ -51,6 +50,7 @@ describe Puppet::Indirector::REST do end it "should not fail" do + Puppet::TestIndirectedFoo.find('bar') lambda { Puppet::TestIndirectedFoo.find('bar') }.should_not raise_error end @@ -62,8 +62,8 @@ describe Puppet::Indirector::REST do Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value end - it 'should set a version timestamp on model instance' do - Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + it 'should set an expiration on model instance' do + Puppet::TestIndirectedFoo.find('bar').expiration.should_not be_nil end end @@ -275,8 +275,8 @@ describe Puppet::Indirector::REST do Puppet::TestIndirectedFoo.find('bar').value.should == @model_instance.value end - it 'should set a version timestamp on model instance' do - Puppet::TestIndirectedFoo.find('bar').version.should_not be_nil + it 'should set an expiration on model instance' do + Puppet::TestIndirectedFoo.find('bar').expiration.should_not be_nil end end @@ -330,11 +330,9 @@ describe Puppet::Indirector::REST do Puppet::TestIndirectedFoo.search('bar').collect(&:value).should == @model_instances.collect(&:value) end - it 'should set a version timestamp on model instances' do - pending("Luke looking at why this version magic might not be working") do - Puppet::TestIndirectedFoo.search('bar').each do |result| - result.version.should_not be_nil - end + it 'should set an expiration on model instances' do + Puppet::TestIndirectedFoo.search('bar').each do |result| + result.expiration.should_not be_nil end end end @@ -453,4 +451,4 @@ describe Puppet::Indirector::REST do @server.unlisten end end -end \ No newline at end of file +end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index e511098ab..e46e32f1c 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -198,45 +198,47 @@ describe Puppet::Indirector::REST do @result = { :foo => 'bar'}.to_yaml @searcher.stubs(:network_fetch).returns(@result) # neuter the network connection @model.stubs(:from_yaml).returns(@instance) + + @request = stub 'request', :key => 'foo' end it "should look up the model instance over the network" do @searcher.expects(:network_fetch).returns(@result) - @searcher.find('foo') + @searcher.find(@request) end it "should look up the model instance using the named indirection" do @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result) - @searcher.find('foo') + @searcher.find(@request) end it "should look up the model instance using the provided key" do @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result) - @searcher.find('foo') + @searcher.find(@request) end it "should deserialize result data to a Model instance" do @model.expects(:from_yaml) - @searcher.find('foo') + @searcher.find(@request) end it "should return the deserialized Model instance" do - @searcher.find('foo').should == @instance + @searcher.find(@request).should == @instance end it "should return nil when deserialized model instance is nil" do @model.stubs(:from_yaml).returns(nil) - @searcher.find('foo').should be_nil + @searcher.find(@request).should be_nil end it "should generate an error when result data deserializes improperly" do @model.stubs(:from_yaml).raises(ArgumentError) - lambda { @searcher.find('foo') }.should raise_error(ArgumentError) + lambda { @searcher.find(@request) }.should raise_error(ArgumentError) end it "should generate an error when result data specifies an error" do @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml) - lambda { @searcher.find('foo') }.should raise_error(RuntimeError) + lambda { @searcher.find(@request) }.should raise_error(RuntimeError) end end @@ -245,36 +247,38 @@ describe Puppet::Indirector::REST do @result = [1, 2].to_yaml @searcher.stubs(:network_fetch).returns(@result) @model.stubs(:from_yaml).returns(@instance) + + @request = stub 'request', :key => 'foo' end it "should look up the model data over the network" do @searcher.expects(:network_fetch).returns(@result) - @searcher.search('foo') + @searcher.search(@request) end it "should look up the model instance using the named indirection" do @searcher.expects(:network_fetch).with {|path| path =~ %r{^#{@indirection.name.to_s}s/} }.returns(@result) - @searcher.search('foo') + @searcher.search(@request) end it "should look up the model instance using the provided key" do @searcher.expects(:network_fetch).with {|path| path =~ %r{/foo$} }.returns(@result) - @searcher.search('foo') + @searcher.search(@request) end it "should deserialize result data into a list of Model instances" do @model.expects(:from_yaml).at_least(2) - @searcher.search('foo') + @searcher.search(@request) end it "should generate an error when result data deserializes improperly" do @model.stubs(:from_yaml).raises(ArgumentError) - lambda { @searcher.search('foo') }.should raise_error(ArgumentError) + lambda { @searcher.search(@request) }.should raise_error(ArgumentError) end it "should generate an error when result data specifies an error" do @searcher.stubs(:network_fetch).returns(RuntimeError.new("bogus").to_yaml) - lambda { @searcher.search('foo') }.should raise_error(RuntimeError) + lambda { @searcher.search(@request) }.should raise_error(RuntimeError) end end @@ -283,35 +287,37 @@ describe Puppet::Indirector::REST do @result = true.to_yaml @searcher.stubs(:network_delete).returns(@result) # neuter the network connection @model.stubs(:from_yaml).returns(@instance) + + @request = stub 'request', :key => 'foo' end it "should look up the model instance over the network" do @searcher.expects(:network_delete).returns(@result) - @searcher.destroy('foo') + @searcher.destroy(@request) end it "should look up the model instance using the named indirection" do @searcher.expects(:network_delete).with {|path| path =~ %r{^#{@indirection.name.to_s}/} }.returns(@result) - @searcher.destroy('foo') + @searcher.destroy(@request) end it "should look up the model instance using the provided key" do @searcher.expects(:network_delete).with {|path| path =~ %r{/foo$} }.returns(@result) - @searcher.destroy('foo') + @searcher.destroy(@request) end it "should deserialize result data" do YAML.expects(:load).with(@result) - @searcher.destroy('foo') + @searcher.destroy(@request) end it "should return deserialized result data" do - @searcher.destroy('foo').should == true + @searcher.destroy(@request).should == true end it "should generate an error when result data specifies an error" do @searcher.stubs(:network_delete).returns(RuntimeError.new("bogus").to_yaml) - lambda { @searcher.destroy('foo') }.should raise_error(RuntimeError) + lambda { @searcher.destroy(@request) }.should raise_error(RuntimeError) end end @@ -320,6 +326,8 @@ describe Puppet::Indirector::REST do @result = { :foo => 'bar'}.to_yaml @searcher.stubs(:network_put).returns(@result) # neuter the network connection @model.stubs(:from_yaml).returns(@instance) + + @request = stub 'request', :instance => @instance end it "should re-enable caching in the terminus" do @@ -328,7 +336,7 @@ describe Puppet::Indirector::REST do it "should save the model instance over the network" do @searcher.expects(:network_put).returns(@result) - @searcher.save(@instance) + @searcher.save(@request) end it "should save the model instance using the named indirection" do @@ -336,31 +344,31 @@ describe Puppet::Indirector::REST do path =~ %r{^#{@indirection.name.to_s}/} and data == @instance.to_yaml end.returns(@result) - @searcher.save(@instance) + @searcher.save(@request) end it "should deserialize result data to a Model instance" do @model.expects(:from_yaml) - @searcher.save(@instance) + @searcher.save(@request) end it "should return the resulting deserialized Model instance" do - @searcher.save(@instance).should == @instance + @searcher.save(@request).should == @instance end it "should return nil when deserialized model instance is nil" do @model.stubs(:from_yaml).returns(nil) - @searcher.save(@instance).should be_nil + @searcher.save(@request).should be_nil end it "should generate an error when result data deserializes improperly" do @model.stubs(:from_yaml).raises(ArgumentError) - lambda { @searcher.save(@instance) }.should raise_error(ArgumentError) + lambda { @searcher.save(@request) }.should raise_error(ArgumentError) end it "should generate an error when result data specifies an error" do @searcher.stubs(:network_put).returns(RuntimeError.new("bogus").to_yaml) - lambda { @searcher.save(@instance) }.should raise_error(RuntimeError) + lambda { @searcher.save(@request) }.should raise_error(RuntimeError) end end end -- cgit From d9846fc3f06f61fcb4b8806740f77747a7f6939e Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 11 Apr 2008 15:04:54 -0500 Subject: Fixishing some pending tests, including filling in the connection information. --- lib/puppet/indirector/rest.rb | 4 ++-- spec/integration/indirector/rest.rb | 10 ++++++++-- spec/unit/indirector/rest.rb | 20 +++++++++----------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 26f7736f3..d33150fc2 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -5,7 +5,7 @@ require 'uri' class Puppet::Indirector::REST < Puppet::Indirector::Terminus def rest_connection_details - { :host => '127.0.0.1', :port => 34343 } + { :host => Puppet[:server], :port => Puppet[:masterport].to_i } end def network_fetch(path) @@ -47,7 +47,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus private def network(&block) - Net::HTTP.start(rest_connection_details[:host], rest_connection_details[:port]) {|conn| yield(conn) } + Net::HTTP.start(rest_connection_details[:host], rest_connection_details[:port]) {|conn| yield(conn) } end def exception?(yaml_string) diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 8e3c92d5c..cafcce713 100644 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -38,7 +38,10 @@ describe Puppet::Indirector::REST do # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) - Puppet::TestIndirectedFoo.terminus_class = :rest + Puppet::TestIndirectedFoo.indirection.stubs(:terminus_class).returns :rest + + # Stub the connection information. + Puppet::TestIndirectedFoo.indirection.terminus(:rest).stubs(:rest_connection_details).returns(:host => "localhost", :port => 34343) end describe "when finding a model instance over REST" do @@ -252,7 +255,10 @@ describe Puppet::Indirector::REST do # the autoloader was clearly not written test-first. We subvert the integration test to get around its bullshit. Puppet::Indirector::Terminus.stubs(:terminus_class).returns(Puppet::TestIndirectedFoo::Rest) - Puppet::TestIndirectedFoo.terminus_class = :rest + Puppet::TestIndirectedFoo.indirection.stubs(:terminus_class).returns :rest + + # Stub the connection information. + Puppet::TestIndirectedFoo.indirection.terminus(:rest).stubs(:rest_connection_details).returns(:host => "localhost", :port => 34343) end describe "when finding a model instance over REST" do diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index e46e32f1c..1a1064491 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -21,16 +21,18 @@ describe Puppet::Indirector::REST do end describe "when locating the REST connection" do - it 'should look somewhere meaningful for connection details' do - pending("Luke enlightening us on where to find the appropriate connection details") + before do + Puppet.settings.stubs(:value).returns("whatever") end - - it "should return a host" do - @searcher.rest_connection_details[:host].should == '127.0.0.1' + + it "should return the :server setting as the host" do + Puppet.settings.expects(:value).with(:server).returns "myserver" + @searcher.rest_connection_details[:host].should == "myserver" end - it "should return a port" do - @searcher.rest_connection_details[:port].should == 34343 + it "should return the :masterport (as an Integer) as the port" do + Puppet.settings.expects(:value).with(:masterport).returns "1234" + @searcher.rest_connection_details[:port].should == 1234 end end @@ -330,10 +332,6 @@ describe Puppet::Indirector::REST do @request = stub 'request', :instance => @instance end - it "should re-enable caching in the terminus" do - pending("Luke taking a look at (a) why that doesn't seem to work with this save, or (b) rewriting it, like he seems to indicate in his blog is going to happen") - end - it "should save the model instance over the network" do @searcher.expects(:network_put).returns(@result) @searcher.save(@request) -- cgit