This commit includes multiple, related changes, all in one commit because the whole thing was necessary to reach a functional tree again:
* The URI starts with the environment, so: /production/certificate/foo /development/file_content/path/to/your/file * All REST handling is done by a single instance mounted at / for webrick and Mongrel, rather than having individual instances mounted at, say, /certificate. * All REST URI translation is done by an API module. Currently only the 'v1' module exists with no support for additional modules, but it's well-separated and will be easy to expand as we need it. Signed-off-by: Luke Kanies <[email protected]> --- lib/puppet/application/puppetd.rb | 2 +- lib/puppet/application/puppetmasterd.rb | 5 +- lib/puppet/network/http/api/v1.rb | 7 ++- lib/puppet/network/http/handler.rb | 32 ++++------ lib/puppet/network/http/mongrel.rb | 13 +--- lib/puppet/network/http/mongrel/rest.rb | 9 +-- lib/puppet/network/http/webrick.rb | 12 +--- lib/puppet/network/http/webrick/rest.rb | 8 +-- spec/integration/indirector/certificate/rest.rb | 2 +- .../indirector/certificate_request/rest.rb | 2 +- .../indirector/certificate_revocation_list/rest.rb | 2 +- spec/integration/indirector/report/rest.rb | 2 +- spec/integration/indirector/rest.rb | 12 ++-- spec/unit/application/puppetd.rb | 6 -- spec/unit/application/puppetmasterd.rb | 7 -- spec/unit/network/http/handler.rb | 66 +++++++++++--------- spec/unit/network/http/mongrel.rb | 23 ++----- spec/unit/network/http/mongrel/rest.rb | 14 +---- spec/unit/network/http/webrick.rb | 27 ++------- spec/unit/network/http/webrick/rest.rb | 14 +---- spec/unit/ssl/certificate.rb | 4 - spec/unit/ssl/certificate_revocation_list.rb | 4 - 22 files changed, 88 insertions(+), 185 deletions(-) diff --git a/lib/puppet/application/puppetd.rb b/lib/puppet/application/puppetd.rb index 3efb0ef..cacb843 100644 --- a/lib/puppet/application/puppetd.rb +++ b/lib/puppet/application/puppetd.rb @@ -177,7 +177,7 @@ Puppet::Application.new(:puppetd) do require 'puppet/network/server' # No REST handlers yet. - server = Puppet::Network::Server.new(:handlers => [:facts], :xmlrpc_handlers => handlers, :port => Puppet[:puppetport]) + server = Puppet::Network::Server.new(:xmlrpc_handlers => handlers, :port => Puppet[:puppetport]) @daemon.server = server end diff --git a/lib/puppet/application/puppetmasterd.rb b/lib/puppet/application/puppetmasterd.rb index 15426ad..fe92bca 100644 --- a/lib/puppet/application/puppetmasterd.rb +++ b/lib/puppet/application/puppetmasterd.rb @@ -55,14 +55,11 @@ Puppet::Application.new(:puppetmasterd) do xmlrpc_handlers = [:Status, :FileServer, :Master, :Report, :Filebucket] - # Just set up serving to all of the indirected classes. - rest_handlers = Puppet::Indirector::Indirection.instances - if Puppet[:ca] xmlrpc_handlers << :CA end - @daemon.server = Puppet::Network::Server.new(:handlers => rest_handlers, :xmlrpc_handlers => xmlrpc_handlers) + @daemon.server = Puppet::Network::Server.new(:xmlrpc_handlers => xmlrpc_handlers) # Make sure we've got a localhost ssl cert Puppet::SSL::Host.localhost diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb index 2ee1a81..13df7c3 100644 --- a/lib/puppet/network/http/api/v1.rb +++ b/lib/puppet/network/http/api/v1.rb @@ -51,7 +51,12 @@ module Puppet::Network::HTTP::API::V1 end def plurality(indirection) - result = (indirection == handler.to_s + "s") ? :plural : :singular + # NOTE This specific hook for facts is ridiculous, but it's a *many*-line + # fix to not need this, and our goal is to move away from the complication + # that leads to the fix being too long. + return :singular if indirection == "facts" + + result = (indirection =~ /s$/) ? :plural : :singular indirection.sub!(/s$/, '') if result diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 76f07ed..db12a8b 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -6,7 +6,7 @@ require 'puppet/network/http/api/v1' module Puppet::Network::HTTP::Handler include Puppet::Network::HTTP::API::V1 - attr_reader :model, :server, :handler + attr_reader :server, :handler # Retrieve the accept header from the http request. def accept_header(request) @@ -30,19 +30,16 @@ module Puppet::Network::HTTP::Handler raise "No specified acceptable formats (%s) are functional on this machine" % header end - def initialize_for_puppet(args = {}) - raise ArgumentError unless @server = args[:server] - raise ArgumentError unless @handler = args[:handler] - @model = find_model_for_handler(@handler) + def initialize_for_puppet(server) + @server = server end # handle an HTTP request def process(request, response) - indirection_request = uri2indirection(path(request), params(request), http_method(request)) + indirection_request = uri2indirection(http_method(request), path(request), params(request)) send("do_%s" % indirection_request.method, indirection_request, request, response) rescue Exception => e - puts e.backtrace return do_exception(response, e) end @@ -67,8 +64,8 @@ module Puppet::Network::HTTP::Handler # Execute our find. def do_find(indirection_request, request, response) - unless result = model.find(indirection_request.key, indirection_request.options) - return do_exception(response, "Could not find %s %s" % [model.name, indirection_request.key], 404) + unless result = indirection_request.model.find(indirection_request.key, indirection_request.to_hash) + return do_exception(response, "Could not find %s %s" % [indirection_request.indirection_name, indirection_request.key], 404) end # The encoding of the result must include the format to use, @@ -82,21 +79,21 @@ module Puppet::Network::HTTP::Handler # Execute our search. def do_search(indirection_request, request, response) - result = model.search(indirection_request.key, indirection_request.options) + result = indirection_request.model.search(indirection_request.key, indirection_request.to_hash) if result.nil? or (result.is_a?(Array) and result.empty?) - return do_exception(response, "Could not find instances in %s with '%s'" % [model.name, indirection_request.options.inspect], 404) + return do_exception(response, "Could not find instances in %s with '%s'" % [indirection_request.indirection_name, indirection_request.to_hash.inspect], 404) end format = format_to_use(request) set_content_type(response, format) - set_response(response, model.render_multiple(format, result)) + set_response(response, indirection_request.model.render_multiple(format, result)) end # Execute our destroy. def do_destroy(indirection_request, request, response) - result = model.destroy(indirection_request.key, indirection_request.options) + result = indirection_request.model.destroy(indirection_request.key, indirection_request.to_hash) set_content_type(response, "yaml") @@ -110,7 +107,7 @@ module Puppet::Network::HTTP::Handler format = format_to_use(request) - obj = model.convert_from(format_to_use(request), data) + obj = indirection_request.model.convert_from(format_to_use(request), data) result = save_object(indirection_request, obj) set_content_type(response, "yaml") @@ -123,12 +120,7 @@ module Puppet::Network::HTTP::Handler # LAK:NOTE This has to be here for testing; it's a stub-point so # we keep infinite recursion from happening. def save_object(ind_request, object) - object.save(ind_request.options) - end - - def find_model_for_handler(handler) - Puppet::Indirector::Indirection.model(handler) || - raise(ArgumentError, "Cannot locate indirection [#{handler}].") + object.save(ind_request.to_hash) end def get?(request) diff --git a/lib/puppet/network/http/mongrel.rb b/lib/puppet/network/http/mongrel.rb index 847781c..3564025 100644 --- a/lib/puppet/network/http/mongrel.rb +++ b/lib/puppet/network/http/mongrel.rb @@ -8,14 +8,12 @@ class Puppet::Network::HTTP::Mongrel 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? 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] @xmlrpc_handlers = args[:xmlrpc_handlers] @server = Mongrel::HttpServer.new(args[:address], args[:port]) setup_handlers @@ -38,14 +36,9 @@ class Puppet::Network::HTTP::Mongrel private def setup_handlers - @protocols.each do |protocol| - next if protocol == :xmlrpc - klass = class_for_protocol(protocol) - @handlers.each do |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 + # Register our REST support at / + klass = class_for_protocol(:rest) + @server.register('/', klass.new(:server => @server)) if @protocols.include?(:xmlrpc) and ! @xmlrpc_handlers.empty? setup_xmlrpc_handlers diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index 04f4139..d9913dc 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -31,13 +31,8 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler # 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('/', 3)[2] + #x = '/' + request.params[Mongrel::Const::REQUEST_PATH] + request.params[Mongrel::Const::REQUEST_PATH] end # return the request body diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb index 972ebc2..46735c0 100644 --- a/lib/puppet/network/http/webrick.rb +++ b/lib/puppet/network/http/webrick.rb @@ -19,13 +19,11 @@ class Puppet::Network::HTTP::WEBrick 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? raise ArgumentError, ":address must be specified." unless args[:address] raise ArgumentError, ":port must be specified." unless args[:port] @protocols = args[:protocols] - @handlers = args[:handlers] @xmlrpc_handlers = args[:xmlrpc_handlers] arguments = {:BindAddress => args[:address], :Port => args[:port]} @@ -115,14 +113,8 @@ class Puppet::Network::HTTP::WEBrick def setup_handlers # Set up the new-style protocols. - @protocols.each do |protocol| - next if protocol == :xmlrpc - klass = self.class.class_for_protocol(protocol) - @handlers.each do |handler| - @server.mount('/' + handler.to_s, klass, handler) - @server.mount('/' + handler.to_s + 's', klass, handler) - end - end + klass = self.class.class_for_protocol(:rest) + @server.mount('/', klass, :this_value_is_apparently_necessary_but_unused) # And then set up xmlrpc, if configured. if @protocols.include?(:xmlrpc) and ! @xmlrpc_handlers.empty? diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb index e34f0d2..8120c87 100644 --- a/lib/puppet/network/http/webrick/rest.rb +++ b/lib/puppet/network/http/webrick/rest.rb @@ -31,13 +31,7 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet end def path(request) - # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] - x = '/' + request.path.split('/')[1] - end - - def request_key(request) - # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] - x = request.path.split('/', 3)[2] + request.path end def body(request) diff --git a/spec/integration/indirector/certificate/rest.rb b/spec/integration/indirector/certificate/rest.rb index 5ddb002..e1f15d9 100755 --- a/spec/integration/indirector/certificate/rest.rb +++ b/spec/integration/indirector/certificate/rest.rb @@ -46,7 +46,7 @@ describe "Certificate REST Terminus" do # passed through REST; otherwise we'd be stubbing 'find', which would cause an immediate # return. @mock_model = stub('faked model', :name => "certificate") - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end after do diff --git a/spec/integration/indirector/certificate_request/rest.rb b/spec/integration/indirector/certificate_request/rest.rb index f8ca1ec..59d2a54 100755 --- a/spec/integration/indirector/certificate_request/rest.rb +++ b/spec/integration/indirector/certificate_request/rest.rb @@ -51,7 +51,7 @@ describe "Certificate Request REST Terminus" do # passed through REST; otherwise we'd be stubbing 'find', which would cause an immediate # return. @mock_model = stub('faked model', :name => "certificate request") - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end after do diff --git a/spec/integration/indirector/certificate_revocation_list/rest.rb b/spec/integration/indirector/certificate_revocation_list/rest.rb index 8df4ad3..5ad2d8c 100755 --- a/spec/integration/indirector/certificate_revocation_list/rest.rb +++ b/spec/integration/indirector/certificate_revocation_list/rest.rb @@ -55,7 +55,7 @@ describe "Certificate REST Terminus" do # passed through REST; otherwise we'd be stubbing 'find', which would cause an immediate # return. @mock_model = stub('faked model', :name => "certificate") - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end after do diff --git a/spec/integration/indirector/report/rest.rb b/spec/integration/indirector/report/rest.rb index 5b37554..6bbfbee 100644 --- a/spec/integration/indirector/report/rest.rb +++ b/spec/integration/indirector/report/rest.rb @@ -48,7 +48,7 @@ describe "Report REST Terminus" do # return. @report = stub_everything 'report' @mock_model = stub_everything 'faked model', :name => "report", :convert_from => @report - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end after do diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index cfaaa50..95fb34b 100755 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -75,7 +75,7 @@ describe Puppet::Indirector::REST do # passed through REST; otherwise we'd be stubbing 'find', which would cause an immediate # return. @mock_model = stub('faked model', :name => "foo") - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end describe "when finding a model instance over REST" do @@ -117,7 +117,7 @@ describe Puppet::Indirector::REST do describe "when no matching model instance can be found" do before :each do @mock_model = stub('faked model', :name => "foo", :find => nil) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end it "should return nil" do @@ -129,7 +129,7 @@ describe Puppet::Indirector::REST do before :each do @mock_model = stub('faked model', :name => "foo") @mock_model.stubs(:find).raises(RuntimeError) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end it "should raise an exception" do @@ -177,7 +177,7 @@ describe Puppet::Indirector::REST do describe "when no matching model instance can be found" do before :each do @mock_model = stub('faked model', :name => "foo", :find => nil) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end it "should return nil" do @@ -189,7 +189,7 @@ describe Puppet::Indirector::REST 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) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end it "should raise an exception" do @@ -305,7 +305,7 @@ describe Puppet::Indirector::REST do # passed through REST; otherwise we'd be stubbing 'find', which would cause an immediate # return. @mock_model = stub('faked model', :name => "foo") - Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:model).returns(@mock_model) + Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) end after do diff --git a/spec/unit/application/puppetd.rb b/spec/unit/application/puppetd.rb index 592b499..d93a527 100755 --- a/spec/unit/application/puppetd.rb +++ b/spec/unit/application/puppetd.rb @@ -398,12 +398,6 @@ describe "puppetd" do @puppetd.setup_listen end - it "should create a server with facts REST handler" do - Puppet::Network::Server.expects(:new).with { |args| args[:handlers] == [:facts] } - - @puppetd.setup_listen - end - it "should use puppet default port" do Puppet.stubs(:[]).with(:puppetport).returns(:port) diff --git a/spec/unit/application/puppetmasterd.rb b/spec/unit/application/puppetmasterd.rb index da1b1dd..5b193eb 100644 --- a/spec/unit/application/puppetmasterd.rb +++ b/spec/unit/application/puppetmasterd.rb @@ -289,13 +289,6 @@ describe "PuppetMaster" do @puppetmasterd.main end - it "should create the server with the right REST handlers" do - Puppet::Indirector::Indirection.stubs(:instances).returns("handlers") - Puppet::Network::Server.expects(:new).with { |args| args[:handlers] == "handlers"} - - @puppetmasterd.main - end - it "should generate a SSL cert for localhost" do Puppet::SSL::Host.expects(:localhost) diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index 85149b6..84b8702 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -21,28 +21,13 @@ describe Puppet::Network::HTTP::Handler do end describe "when initializing" do - before do - Puppet::Indirector::Indirection.stubs(:model).returns "eh" - end - it "should fail when no server type has been provided" do - lambda { @handler.initialize_for_puppet :handler => "foo" }.should raise_error(ArgumentError) + lambda { @handler.initialize_for_puppet }.should raise_error(ArgumentError) end - it "should fail when no handler has been provided" do - lambda { @handler.initialize_for_puppet :server => "foo" }.should raise_error(ArgumentError) - end - - it "should set the handler and server type" do - @handler.initialize_for_puppet :server => "foo", :handler => "bar" + it "should set server type" do + @handler.initialize_for_puppet("foo") @handler.server.should == "foo" - @handler.handler.should == "bar" - end - - it "should use the indirector to find the appropriate model" do - Puppet::Indirector::Indirection.expects(:model).with("bar").returns "mymodel" - @handler.initialize_for_puppet :server => "foo", :handler => "bar" - @handler.model.should == "mymodel" end end @@ -59,9 +44,6 @@ describe Puppet::Network::HTTP::Handler do @result = stub 'result', :render => "mytext" - @handler.stubs(:model).returns @model_class - @handler.stubs(:handler).returns :my_handler - stub_server_interface end @@ -82,7 +64,7 @@ describe Puppet::Network::HTTP::Handler do @handler.expects(:http_method).with(@request).returns "mymethod" @handler.expects(:params).with(@request).returns "myparams" - @handler.expects(:uri2indirection).with("mypath", "myparams", "mymethod").returns stub("request", :method => :find) + @handler.expects(:uri2indirection).with("mymethod", "mypath", "myparams").returns stub("request", :method => :find) @handler.stubs(:do_find) @@ -115,7 +97,7 @@ describe Puppet::Network::HTTP::Handler do describe "when finding a model instance" do before do - @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :options => {}, :key => "my_result" + @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class @model_class.stubs(:find).returns @result @@ -123,6 +105,12 @@ describe Puppet::Network::HTTP::Handler do Puppet::Network::FormatHandler.stubs(:format).returns @format end + it "should use the indirection request to find the model class" do + @irequest.expects(:model).returns @model_class + + @handler.do_find(@irequest, @request, @response) + end + it "should use the escaped request key" do @model_class.expects(:find).with do |key, args| key == "my_result" @@ -131,7 +119,7 @@ describe Puppet::Network::HTTP::Handler do end it "should use a common method for determining the request parameters" do - @irequest.stubs(:options).returns(:foo => :baz, :bar => :xyzzy) + @irequest.stubs(:to_hash).returns(:foo => :baz, :bar => :xyzzy) @model_class.expects(:find).with do |key, args| args[:foo] == :baz and args[:bar] == :xyzzy end.returns @result @@ -206,7 +194,7 @@ describe Puppet::Network::HTTP::Handler do describe "when searching for model instances" do before do - @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :options => {}, :key => "key" + @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class @result1 = mock 'result1' @result2 = mock 'results' @@ -219,8 +207,14 @@ describe Puppet::Network::HTTP::Handler do Puppet::Network::FormatHandler.stubs(:format).returns @format end + it "should use the indirection request to find the model" do + @irequest.expects(:model).returns @model_class + + @handler.do_search(@irequest, @request, @response) + end + it "should use a common method for determining the request parameters" do - @irequest.stubs(:options).returns(:foo => :baz, :bar => :xyzzy) + @irequest.stubs(:to_hash).returns(:foo => :baz, :bar => :xyzzy) @model_class.expects(:search).with do |key, args| args[:foo] == :baz and args[:bar] == :xyzzy end.returns @result @@ -267,12 +261,18 @@ describe Puppet::Network::HTTP::Handler do describe "when destroying a model instance" do before do - @irequest = stub 'indirection_request', :method => :destroy, :indirection_name => "my_handler", :options => {}, :key => "key" + @irequest = stub 'indirection_request', :method => :destroy, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class @result = stub 'result', :render => "the result" @model_class.stubs(:destroy).returns @result end + it "should use the indirection request to find the model" do + @irequest.expects(:model).returns @model_class + + @handler.do_destroy(@irequest, @request, @response) + end + it "should use the escaped request key to destroy the instance in the model" do @irequest.expects(:key).returns "foo bar" @model_class.expects(:destroy).with do |key, args| @@ -282,7 +282,7 @@ describe Puppet::Network::HTTP::Handler do end it "should use a common method for determining the request parameters" do - @irequest.stubs(:options).returns(:foo => :baz, :bar => :xyzzy) + @irequest.stubs(:to_hash).returns(:foo => :baz, :bar => :xyzzy) @model_class.expects(:destroy).with do |key, args| args[:foo] == :baz and args[:bar] == :xyzzy end @@ -306,7 +306,7 @@ describe Puppet::Network::HTTP::Handler do describe "when saving a model instance" do before do - @irequest = stub 'indirection_request', :method => :save, :indirection_name => "my_handler", :options => {}, :key => "key" + @irequest = stub 'indirection_request', :method => :save, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class @handler.stubs(:body).returns('my stuff') @result = stub 'result', :render => "the result" @@ -318,6 +318,12 @@ describe Puppet::Network::HTTP::Handler do Puppet::Network::FormatHandler.stubs(:format).returns @format end + it "should use the indirection request to find the model" do + @irequest.expects(:model).returns @model_class + + @handler.do_save(@irequest, @request, @response) + end + it "should use the 'body' hook to retrieve the body of the request" do @handler.expects(:body).returns "my body" @model_class.expects(:convert_from).with { |format, body| body == "my body" }.returns @model_instance @@ -332,7 +338,7 @@ describe Puppet::Network::HTTP::Handler do end it "should use a common method for determining the request parameters" do - @irequest.stubs(:options).returns(:foo => :baz, :bar => :xyzzy) + @irequest.stubs(:to_hash).returns(:foo => :baz, :bar => :xyzzy) @model_instance.expects(:save).with do |args| args[:foo] == :baz and args[:bar] == :xyzzy end diff --git a/spec/unit/network/http/mongrel.rb b/spec/unit/network/http/mongrel.rb index 9c70806..59f8932 100755 --- a/spec/unit/network/http/mongrel.rb +++ b/spec/unit/network/http/mongrel.rb @@ -31,10 +31,7 @@ describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do @mock_puppet_mongrel = mock('puppet_mongrel') Puppet::Network::HTTPServer::Mongrel.stubs(:new).returns(@mock_puppet_mongrel) - @listen_params = { :address => "127.0.0.1", :port => 31337, - :handlers => [ :node, :catalog ], :protocols => [ :rest, :xmlrpc ], - :xmlrpc_handlers => [ :status, :fileserver ] - } + @listen_params = { :address => "127.0.0.1", :port => 31337, :protocols => [ :rest, :xmlrpc ], :xmlrpc_handlers => [ :status, :fileserver ] } end it "should fail if already listening" do @@ -42,10 +39,6 @@ describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do Proc.new { @server.listen(@listen_params) }.should raise_error(RuntimeError) end - it "should require at least one handler" do - Proc.new { @server.listen(@listen_params.delete_if {|k,v| :handlers == k}) }.should raise_error(ArgumentError) - end - it "should require at least one protocol" do Proc.new { @server.listen(@listen_params.delete_if {|k,v| :protocols == k}) }.should raise_error(ArgumentError) end @@ -75,12 +68,10 @@ describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do end describe "when providing REST services" do - it "should instantiate a handler for each protocol+handler pair to configure web server routing" do - @listen_params[:protocols].each do |protocol| - @listen_params[:handlers].each do |handler| - @mock_mongrel.expects(:register) - end - end + it "should instantiate a handler at / for handling REST calls" do + Puppet::Network::HTTP::MongrelREST.expects(:new).returns "myhandler" + @mock_mongrel.expects(:register).with("/", "myhandler") + @server.listen(@listen_params) end @@ -107,10 +98,6 @@ describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do @server.listen(@listen_params.merge(:xmlrpc_handlers => [:status, :master])) end 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 c03698d..84a7e7f 100755 --- a/spec/unit/network/http/mongrel/rest.rb +++ b/spec/unit/network/http/mongrel/rest.rb @@ -48,19 +48,9 @@ describe "Puppet::Network::HTTP::MongrelREST" do @handler.http_method(@request).should == "mymethod" end - it "should use the first part of the request path as the path" do + it "should return the request path as the path" do @params.expects(:[]).with(Mongrel::Const::REQUEST_PATH).returns "/foo/bar" - @handler.path(@request).should == "/foo" - end - - it "should use the remainder of the request path as the request key" do - @params.expects(:[]).with(Mongrel::Const::REQUEST_PATH).returns "/foo/bar/baz" - @handler.request_key(@request).should == "bar/baz" - end - - it "should return nil as the request key if no second field is present" do - @params.expects(:[]).with(Mongrel::Const::REQUEST_PATH).returns "/foo" - @handler.request_key(@request).should be_nil + @handler.path(@request).should == "/foo/bar" end it "should return the request body as the body" do diff --git a/spec/unit/network/http/webrick.rb b/spec/unit/network/http/webrick.rb index e02d5c2..a6fbfc8 100755 --- a/spec/unit/network/http/webrick.rb +++ b/spec/unit/network/http/webrick.rb @@ -20,9 +20,7 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do WEBrick::HTTPServer.stubs(:new).returns(@mock_webrick) @server = Puppet::Network::HTTP::WEBrick.new [:setup_logger, :setup_ssl].each {|meth| @server.stubs(meth).returns({})} # the empty hash is required because of how we're merging - @listen_params = { :address => "127.0.0.1", :port => 31337, - :handlers => [ :node, :catalog ], :xmlrpc_handlers => [], :protocols => [ :rest ] - } + @listen_params = { :address => "127.0.0.1", :port => 31337, :xmlrpc_handlers => [], :protocols => [ :rest ] } end it "should fail if already listening" do @@ -30,10 +28,6 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do Proc.new { @server.listen(@listen_params) }.should raise_error(RuntimeError) end - it "should require at least one handler" do - Proc.new { @server.listen(@listen_params.delete_if {|k,v| :handlers == k}) }.should raise_error(ArgumentError) - end - it "should require at least one protocol" do Proc.new { @server.listen(@listen_params.delete_if {|k,v| :protocols == k}) }.should raise_error(ArgumentError) end @@ -84,18 +78,11 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do end describe "when the REST protocol is requested" do - it "should use a WEBrick + REST class to configure WEBrick" do - Puppet::Network::HTTP::WEBrick.expects(:class_for_protocol).with(:rest).at_least_once - @server.listen(@listen_params.merge(:protocols => [:rest])) - end + it "should register the REST handler at /" do + # We don't care about the options here. + @mock_webrick.expects(:mount).with { |path, klass, options| path == "/" and klass == Puppet::Network::HTTP::WEBrickREST } - it "should instantiate a handler for each protocol+handler pair to configure web server routing" do - @listen_params[:protocols].each do |protocol| - @listen_params[:handlers].each do |handler| - @mock_webrick.expects(:mount) - end - end - @server.listen(@listen_params) + @server.listen(@listen_params.merge(:protocols => [:rest])) end end @@ -150,10 +137,6 @@ describe Puppet::Network::HTTP::WEBrick, "when turning on listening" do @server.listen(@listen_params.merge(:protocols => [:xmlrpc], :xmlrpc_handlers => [:master, :fileserver])) end 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 - end end diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb index b32cf49..bb09181 100755 --- a/spec/unit/network/http/webrick/rest.rb +++ b/spec/unit/network/http/webrick/rest.rb @@ -42,19 +42,9 @@ describe Puppet::Network::HTTP::WEBrickREST do @handler.http_method(@request).should == "FOO" end - it "should return the first argument of the request path as the path" do + it "should return the request path as the path" do @request.expects(:path).returns "/foo/bar" - @handler.path(@request).should == "/foo" - end - - it "should return the remainder of the path as the request key" do - @request.expects(:path).returns "/foo/bar/baz" - @handler.request_key(@request).should == "bar/baz" - end - - it "should return nil as the request key if there is no second field" do - @request.expects(:path).returns "/foo" - @handler.request_key(@request).should be_nil + @handler.path(@request).should == "/foo/bar" end it "should return the request body as the body" do diff --git a/spec/unit/ssl/certificate.rb b/spec/unit/ssl/certificate.rb index a092c6a..0d48991 100755 --- a/spec/unit/ssl/certificate.rb +++ b/spec/unit/ssl/certificate.rb @@ -21,10 +21,6 @@ describe Puppet::SSL::Certificate do @class.indirection.name.should == :certificate end - it "should default to the :file terminus" do - @class.indirection.terminus_class.should == :file - end - it "should only support the text format" do @class.supported_formats.should == [:s] end diff --git a/spec/unit/ssl/certificate_revocation_list.rb b/spec/unit/ssl/certificate_revocation_list.rb index 3963b40..eb25268 100755 --- a/spec/unit/ssl/certificate_revocation_list.rb +++ b/spec/unit/ssl/certificate_revocation_list.rb @@ -12,10 +12,6 @@ describe Puppet::SSL::CertificateRevocationList do @class = Puppet::SSL::CertificateRevocationList end - it "should default to the :file terminus" do - @class.indirection.terminus_class.should == :file - end - it "should only support the text format" do @class.supported_formats.should == [:s] end -- 1.6.1 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
