Merge branch 'maint/next/remove-request-object-from-rest-api' into next

4 files changed, 59 insertions, 138 deletions

diff --git a/spec/unit/network/http/api/v1_spec.rb b/spec/unit/network/http/api/v1_spec.rb
index 8d507046f..84b98ddaf 100644
--- a/spec/unit/network/http/api/v1_spec.rb
+++ b/spec/unit/network/http/api/v1_spec.rb
@@ -32,7 +32,7 @@ describe Puppet::Network::HTTP::API::V1 do
     end
 
     it "should use the first field of the URI as the environment" do
-      @tester.uri2indirection("GET", "/env/foo/bar", {}).environment.should == Puppet::Node::Environment.new("env")
+      @tester.uri2indirection("GET", "/env/foo/bar", {})[3][:environment].should == "env"
     end
 
     it "should fail if the environment is not alphanumeric" do
@@ -40,11 +40,11 @@ describe Puppet::Network::HTTP::API::V1 do
     end
 
     it "should use the environment from the URI even if one is specified in the parameters" do
-      @tester.uri2indirection("GET", "/env/foo/bar", {:environment => "otherenv"}).environment.should == Puppet::Node::Environment.new("env")
+      @tester.uri2indirection("GET", "/env/foo/bar", {:environment => "otherenv"})[3][:environment].should == "env"
     end
 
     it "should use the second field of the URI as the indirection name" do
-      @tester.uri2indirection("GET", "/env/foo/bar", {}).indirection_name.should == :foo
+      @tester.uri2indirection("GET", "/env/foo/bar", {})[0].should == "foo"
     end
 
     it "should fail if the indirection name is not alphanumeric" do
@@ -52,11 +52,11 @@ describe Puppet::Network::HTTP::API::V1 do
     end
 
     it "should use the remainder of the URI as the indirection key" do
-      @tester.uri2indirection("GET", "/env/foo/bar", {}).key.should == "bar"
+      @tester.uri2indirection("GET", "/env/foo/bar", {})[2].should == "bar"
     end
 
     it "should support the indirection key being a /-separated file path" do
-      @tester.uri2indirection("GET", "/env/foo/bee/baz/bomb", {}).key.should == "bee/baz/bomb"
+      @tester.uri2indirection("GET", "/env/foo/bee/baz/bomb", {})[2].should == "bee/baz/bomb"
     end
 
     it "should fail if no indirection key is specified" do
@@ -65,31 +65,31 @@ describe Puppet::Network::HTTP::API::V1 do
     end
 
     it "should choose 'find' as the indirection method if the http method is a GET and the indirection name is singular" do
-      @tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find
+      @tester.uri2indirection("GET", "/env/foo/bar", {})[1].should == :find
     end
 
     it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do
-      @tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search
+      @tester.uri2indirection("GET", "/env/foos/bar", {})[1].should == :search
     end
 
     it "should choose 'find' as the indirection method if the http method is a GET and the indirection name is facts" do
-      @tester.uri2indirection("GET", "/env/facts/bar", {}).method.should == :find
+      @tester.uri2indirection("GET", "/env/facts/bar", {})[1].should == :find
     end
 
     it "should choose 'save' as the indirection method if the http method is a PUT and the indirection name is facts" do
-      @tester.uri2indirection("PUT", "/env/facts/bar", {}).method.should == :save
+      @tester.uri2indirection("PUT", "/env/facts/bar", {})[1].should == :save
     end
 
     it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is inventory" do
-      @tester.uri2indirection("GET", "/env/inventory/search", {}).method.should == :search
+      @tester.uri2indirection("GET", "/env/inventory/search", {})[1].should == :search
     end
 
     it "should choose 'delete' as the indirection method if the http method is a DELETE and the indirection name is singular" do
-      @tester.uri2indirection("DELETE", "/env/foo/bar", {}).method.should == :destroy
+      @tester.uri2indirection("DELETE", "/env/foo/bar", {})[1].should == :destroy
     end
 
     it "should choose 'save' as the indirection method if the http method is a PUT and the indirection name is singular" do
-      @tester.uri2indirection("PUT", "/env/foo/bar", {}).method.should == :save
+      @tester.uri2indirection("PUT", "/env/foo/bar", {})[1].should == :save
     end
 
     it "should fail if an indirection method cannot be picked" do
@@ -98,7 +98,8 @@ describe Puppet::Network::HTTP::API::V1 do
 
     it "should URI unescape the indirection key" do
       escaped = URI.escape("foo bar")
-      @tester.uri2indirection("GET", "/env/foo/#{escaped}", {}).key.should == "foo bar"
+      indirection_name, method, key, params = @tester.uri2indirection("GET", "/env/foo/#{escaped}", {})
+      key.should == "foo bar"
     end
   end
 
diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
index 355f500e6..16f1b5349 100755
--- a/spec/unit/network/http/handler_spec.rb
+++ b/spec/unit/network/http/handler_spec.rb
@@ -79,37 +79,22 @@ describe Puppet::Network::HTTP::Handler do
       @handler.process(@request, @response)
     end
 
-    it "should call the 'do' method associated with the indirection method" do
-      request = stub 'request'
-      @handler.expects(:uri2indirection).returns request
+    it "should call the 'do' method and delegate authorization to the RestAuthorization layer" do
+      @handler.expects(:uri2indirection).returns(["facts", :mymethod, "key", {:node => "name"}])
 
-      request.expects(:method).returns "mymethod"
+      @handler.expects(:do_mymethod).with("facts", "key", {:node => "name"}, @request, @response)
 
-      @handler.expects(:do_mymethod).with(request, @request, @response)
-
-      @handler.process(@request, @response)
-    end
-
-    it "should delegate authorization to the RestAuthorization layer" do
-      request = stub 'request'
-      @handler.expects(:uri2indirection).returns request
-
-      request.expects(:method).returns "mymethod"
-
-      @handler.expects(:do_mymethod).with(request, @request, @response)
-
-      @handler.expects(:check_authorization).with(request)
+      @handler.expects(:check_authorization).with("facts", :mymethod, "key", {:node => "name"})
 
       @handler.process(@request, @response)
     end
 
     it "should return 403 if the request is not authorized" do
-      request = stub 'request'
-      @handler.expects(:uri2indirection).returns request
+      @handler.expects(:uri2indirection).returns(["facts", :mymethod, "key", {:node => "name"}])
 
       @handler.expects(:do_mymethod).never
 
-      @handler.expects(:check_authorization).with(request).raises(Puppet::Network::AuthorizationError.new("forbindden"))
+      @handler.expects(:check_authorization).with("facts", :mymethod, "key", {:node => "name"}).raises(Puppet::Network::AuthorizationError.new("forbidden"))
 
       @handler.expects(:set_response).with { |response, body, status| status == 403 }
 
@@ -117,7 +102,7 @@ describe Puppet::Network::HTTP::Handler do
     end
 
     it "should serialize a controller exception when an exception is thrown while finding the model instance" do
-      @handler.expects(:uri2indirection).returns stub("request", :method => :find)
+      @handler.expects(:uri2indirection).returns(["facts", :find, "key", {:node => "name"}])
 
       @handler.expects(:do_find).raises(ArgumentError, "The exception")
       @handler.expects(:set_response).with { |response, body, status| body == "The exception" and status == 400 }
@@ -141,9 +126,8 @@ 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", :to_hash => {}, :key => "my_result", :model => @model_class
-
         @model_class.stubs(:find).returns @result
+        Puppet::Indirector::Indirection.expects(:instance).with(:my_handler).returns( stub "indirection", :model => @model_class )
 
         @format = stub 'format', :suitable? => true, :mime => "text/format", :name => "format"
         Puppet::Network::FormatHandler.stubs(:format).returns @format
@@ -153,40 +137,37 @@ describe Puppet::Network::HTTP::Handler do
       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)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should use the escaped request key" do
         @model_class.expects(:find).with do |key, args|
           key == "my_result"
         end.returns @result
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should use a common method for determining the request parameters" do
-        @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
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {:foo => :baz, :bar => :xyzzy}, @request, @response)
       end
 
       it "should set the content type to the first format specified in the accept header" do
         @handler.expects(:accept_header).with(@request).returns "one,two"
         @handler.expects(:set_content_type).with(@response, @oneformat)
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should fail if no accept header is provided" do
         @handler.expects(:accept_header).with(@request).returns nil
-        lambda { @handler.do_find(@irequest, @request, @response) }.should raise_error(ArgumentError)
+        lambda { @handler.do_find("my_handler", "my_result", {}, @request, @response) }.should raise_error(ArgumentError)
       end
 
       it "should fail if the accept header does not contain a valid format" do
         @handler.expects(:accept_header).with(@request).returns ""
-        lambda { @handler.do_find(@irequest, @request, @response) }.should raise_error(RuntimeError)
+        lambda { @handler.do_find("my_handler", "my_result", {}, @request, @response) }.should raise_error(RuntimeError)
       end
 
       it "should not use an unsuitable format" do
@@ -198,7 +179,7 @@ describe Puppet::Network::HTTP::Handler do
 
         @handler.expects(:set_content_type).with(@response, bar) # the suitable one
 
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should render the result using the first format specified in the accept header" do
@@ -206,12 +187,12 @@ describe Puppet::Network::HTTP::Handler do
         @handler.expects(:accept_header).with(@request).returns "one,two"
         @result.expects(:render).with(@oneformat)
 
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should use the default status when a model find call succeeds" do
         @handler.expects(:set_response).with { |response, body, status| status.nil? }
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return a serialized object when a model find call succeeds" do
@@ -220,14 +201,14 @@ describe Puppet::Network::HTTP::Handler do
 
         @handler.expects(:set_response).with { |response, body, status| body == "my_rendered_object" }
         @model_class.stubs(:find).returns(@model_instance)
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return a 404 when no model instance can be found" do
         @model_class.stubs(:name).returns "my name"
         @handler.expects(:set_response).with { |response, body, status| status == 404 }
         @model_class.stubs(:find).returns(nil)
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should write a log message when no model instance can be found" do
@@ -236,7 +217,7 @@ describe Puppet::Network::HTTP::Handler do
 
         Puppet.expects(:info).with("Could not find my_handler for 'my_result'")
 
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
 
 
@@ -246,13 +227,13 @@ describe Puppet::Network::HTTP::Handler do
         @handler.expects(:format_to_use).returns(@oneformat)
         @model_instance.expects(:render).with(@oneformat).returns "my_rendered_object"
         @model_class.stubs(:find).returns(@model_instance)
-        @handler.do_find(@irequest, @request, @response)
+        @handler.do_find("my_handler", "my_result", {}, @request, @response)
       end
     end
 
     describe "when searching for model instances" do
       before do
-        @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
+        Puppet::Indirector::Indirection.expects(:instance).with(:my_handler).returns( stub "indirection", :model => @model_class )
 
         @result1 = mock 'result1'
         @result2 = mock 'results'
@@ -269,29 +250,26 @@ describe Puppet::Network::HTTP::Handler do
       end
 
       it "should use the indirection request to find the model" do
-        @irequest.expects(:model).returns @model_class
-
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should use a common method for determining the request parameters" do
-        @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
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {:foo => :baz, :bar => :xyzzy}, @request, @response)
       end
 
       it "should use the default status when a model search call succeeds" do
         @model_class.stubs(:search).returns(@result)
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should set the content type to the first format returned by the accept header" do
         @handler.expects(:accept_header).with(@request).returns "one,two"
         @handler.expects(:set_content_type).with(@response, @oneformat)
 
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return a list of serialized objects when a model search call succeeds" do
@@ -302,7 +280,7 @@ describe Puppet::Network::HTTP::Handler do
         @model_class.expects(:render_multiple).with(@oneformat, @result).returns "my rendered instances"
 
         @handler.expects(:set_response).with { |response, data| data == "my rendered instances" }
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return [] when searching returns an empty array" do
@@ -312,50 +290,46 @@ describe Puppet::Network::HTTP::Handler do
 
 
         @handler.expects(:set_response).with { |response, data| data == "[]" }
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return a 404 when searching returns nil" do
         @model_class.stubs(:name).returns "my name"
         @handler.expects(:set_response).with { |response, body, status| status == 404 }
         @model_class.stubs(:search).returns(nil)
-        @handler.do_search(@irequest, @request, @response)
+        @handler.do_search("my_handler", "my_result", {}, @request, @response)
       end
     end
 
     describe "when destroying a model instance" do
       before do
-        @irequest = stub 'indirection_request', :method => :destroy, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
+        Puppet::Indirector::Indirection.expects(:instance).with(:my_handler).returns( stub "indirection", :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)
+        @handler.do_destroy("my_handler", "my_result", {}, @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|
           key == "foo bar"
         end
-        @handler.do_destroy(@irequest, @request, @response)
+        @handler.do_destroy("my_handler", "foo bar", {}, @request, @response)
       end
 
       it "should use a common method for determining the request parameters" do
-        @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
-        @handler.do_destroy(@irequest, @request, @response)
+        @handler.do_destroy("my_handler", "my_result", {:foo => :baz, :bar => :xyzzy}, @request, @response)
       end
 
       it "should use the default status code a model destroy call succeeds" do
         @handler.expects(:set_response).with { |response, body, status| status.nil? }
-        @handler.do_destroy(@irequest, @request, @response)
+        @handler.do_destroy("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return a yaml-encoded result when a model destroy call succeeds" do
@@ -364,13 +338,13 @@ describe Puppet::Network::HTTP::Handler do
 
         @handler.expects(:set_response).with { |response, body, status| body == "the result" }
 
-        @handler.do_destroy(@irequest, @request, @response)
+        @handler.do_destroy("my_handler", "my_result", {}, @request, @response)
       end
     end
 
     describe "when saving a model instance" do
       before do
-        @irequest = stub 'indirection_request', :method => :save, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
+        Puppet::Indirector::Indirection.stubs(:instance).with(:my_handler).returns( stub "indirection", :model => @model_class )
         @handler.stubs(:body).returns('my stuff')
         @handler.stubs(:content_type_header).returns("text/yaml")
 
@@ -386,43 +360,41 @@ describe Puppet::Network::HTTP::Handler do
       end
 
       it "should use the indirection request to find the model" do
-        @irequest.expects(:model).returns @model_class
-
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @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
 
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should fail to save model if data is not specified" do
         @handler.stubs(:body).returns('')
 
-        lambda { @handler.do_save(@irequest, @request, @response) }.should raise_error(ArgumentError)
+        lambda { @handler.do_save("my_handler", "my_result", {}, @request, @response) }.should raise_error(ArgumentError)
       end
 
       it "should use a common method for determining the request parameters" do
         @model_instance.expects(:save).with('key').once
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "key", {}, @request, @response)
       end
 
       it "should use the default status when a model save call succeeds" do
         @handler.expects(:set_response).with { |response, body, status| status.nil? }
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should return the yaml-serialized result when a model save call succeeds" do
         @model_instance.stubs(:save).returns(@model_instance)
         @model_instance.expects(:to_yaml).returns('foo')
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should set the content to yaml" do
         @handler.expects(:set_content_type).with(@response, @yamlformat)
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @request, @response)
       end
 
       it "should use the content-type header to know the body format" do
@@ -431,7 +403,7 @@ describe Puppet::Network::HTTP::Handler do
 
         @model_class.expects(:convert_from).with { |format, body| format == "format" }.returns @model_instance
 
-        @handler.do_save(@irequest, @request, @response)
+        @handler.do_save("my_handler", "my_result", {}, @request, @response)
       end
     end
   end
diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
index 351f3f040..e81eb41ed 100755
--- a/spec/unit/network/rest_authconfig_spec.rb
+++ b/spec/unit/network/rest_authconfig_spec.rb
@@ -30,9 +30,6 @@ describe Puppet::Network::RestAuthConfig do
 
     @acl = stub_everything 'rights'
     @authconfig.rights = @acl
-
-    @request = stub 'request', :indirection_name => "path", :key => "to/resource", :ip => "127.0.0.1",
-      :node => "me", :method => :save, :environment => :env, :authenticated => true
   end
 
   it "should use the puppet default rest authorization file" do
@@ -41,16 +38,10 @@ describe Puppet::Network::RestAuthConfig do
     Puppet::Network::RestAuthConfig.new(nil, false)
   end
 
-  it "should read the config file when needed" do
-    @authconfig.expects(:read)
-
-    @authconfig.allowed?(@request)
-  end
-
   it "should ask for authorization to the ACL subsystem" do
     @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true)
 
-    @authconfig.allowed?(@request)
+    @authconfig.allowed?("path", :save, "to/resource", :ip => "127.0.0.1", :node => "me", :environment => :env, :authenticated => true)
   end
 
   describe "when defining an acl with mk_acl" do
diff --git a/spec/unit/network/rest_authorization_spec.rb b/spec/unit/network/rest_authorization_spec.rb
deleted file mode 100755
index 0cb0bcee9..000000000
--- a/spec/unit/network/rest_authorization_spec.rb
+++ /dev/null
@@ -1,43 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../spec_helper'
-
-require 'puppet/network/rest_authorization'
-
-class RestAuthorized
-  include Puppet::Network::RestAuthorization
-end
-
-
-describe Puppet::Network::RestAuthorization do
-  before :each do
-    @auth = RestAuthorized.new
-    @authconig = stub 'authconfig'
-    @auth.stubs(:authconfig).returns(@authconfig)
-
-    @request = stub_everything 'request'
-    @request.stubs(:method).returns(:find)
-    @request.stubs(:node).returns("node")
-    @request.stubs(:ip).returns("ip")
-  end
-
-  describe "when testing request authorization" do
-    it "should delegate to the current rest authconfig" do
-      @authconfig.expects(:allowed?).with(@request).returns(true)
-
-      @auth.check_authorization(@request)
-    end
-
-    it "should raise an AuthorizationError if authconfig raises an AuthorizationError" do
-      @authconfig.expects(:allowed?).with(@request).raises(Puppet::Network::AuthorizationError.new("forbidden"))
-
-      lambda { @auth.check_authorization(@request) }.should raise_error(Puppet::Network::AuthorizationError)
-    end
-
-    it "should not raise an AuthorizationError if request is allowed" do
-      @authconfig.expects(:allowed?).with(@request).returns(true)
-
-      lambda { @auth.check_authorization(@request) }.should_not raise_error(Puppet::Network::AuthorizationError)
-    end
-  end
-end