Inlined the controller, eliminating a class. Mongrel+REST has the right bits for request handling prior to the encode/decode/exception-handling bits. Refactored to make the common logic extractable to a base class.

5 files changed, 96 insertions, 85 deletions

diff --git a/lib/puppet/network/controller.rb b/lib/puppet/network/controller.rb
deleted file mode 100644
index 7e4cca643..000000000
--- a/lib/puppet/network/controller.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-class Puppet::Network::Controller
-    def initialize(args = {})
-        raise ArgumentError, ":indirection is required" unless args[:indirection]
-        @indirection = args[:indirection]
-        @klass = model_class_from_indirection_name(@indirection)
-    end
-    
-    def find(args = {})
-        @klass.find(args)
-    end
-    
-    def destroy(args = {})
-        @klass.destroy(args)
-    end
-    
-    def search(args = {})
-        @klass.search(args)
-    end
-
-    def save(args = {})
-        instance = @klass.new(args)
-        instance.save
-    end
-    
-  private
-    
-    def model_class_from_indirection_name
-        Class.new # TODO : FIXME make this the indirection class
-    end
-end
diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb
index 452dafa85..f9a6e3796 100644
--- a/lib/puppet/network/http/mongrel/rest.rb
+++ b/lib/puppet/network/http/mongrel/rest.rb
@@ -5,16 +5,55 @@ class Puppet::Network::HTTP::MongrelREST
         register_handler
     end
     
+    # handle an HTTP request coming from Mongrel
+    def process(request, response)
+        return @model.find     if get?(request)    and singular?(request)
+        return @model.search   if get?(request)    and plural?(request)
+        return @model.destroy  if delete?(request) and singular?(request)
+        return @model.new.save if put?(request) and singular?(request)
+        raise ArgumentError, "Did not understand HTTP #{http_method(request)} request for '#{path(request)}'"
+        # TODO: here, raise an exception, or do some defaulting or something
+    end
+    
   private
     
+    def find_model_for_handler(handler)
+        Puppet::Indirector::Indirection.model(handler) || 
+            raise(ArgumentError, "Cannot locate indirection [#{handler}].")
+    end
+    
+    def get?(request)
+        http_method(request) == 'GET'
+    end
+    
+    def put?(request)
+        http_method(request) == 'PUT'
+    end
+    
+    def delete?(request)
+        http_method(request) == 'DELETE'
+    end
+    
+    def singular?(request)
+        %r{/#{@handler.to_s}$}.match(path(request))
+    end
+    
+    def plural?(request)
+        %r{/#{@handler.to_s}s$}.match(path(request))
+    end
+    
     def register_handler
         @model = find_model_for_handler(@handler)
         @server.register('/' + @handler.to_s, self)
         @server.register('/' + @handler.to_s + 's', self)
     end
     
-    def find_model_for_handler(handler)
-        Puppet::Indirector::Indirection.model(handler) || 
-            raise(ArgumentError, "Cannot locate indirection [#{handler}].")
+    def http_method(request)
+        request.params[Mongrel::Const::REQUEST_METHOD]
     end
+    
+    def path(request)
+        request.params[Mongrel::Const::REQUEST_PATH]
+    end
+    
 end
diff --git a/spec/unit/network/controller.rb b/spec/unit/network/controller.rb
deleted file mode 100644
index 9098b6e25..000000000
--- a/spec/unit/network/controller.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/usr/bin/env ruby
-#
-#  Created by Rick Bradley on 2007-10-03.
-#  Copyright (c) 2007. All rights reserved.
-
-require File.dirname(__FILE__) + '/../../spec_helper'
-
-require 'puppet/network/controller'
-
-describe Puppet::Network::Controller, "when initializing" do
-    it "should require an indirection name" do
-        Proc.new { Puppet::Network::Controller.new }.should raise_error(ArgumentError)
-    end
-end
-
-describe Puppet::Network::Controller, "after initialization" do
-    before do
-        @mock_model_class = mock('model class')
-        Puppet::Network::Controller.any_instance.stubs(:model_class_from_indirection_name).returns(@mock_model_class)
-        @controller = Puppet::Network::Controller.new(:indirection => :foo)
-    end
-
-    it "should delegate find to the indirection's model class's find" do
-        @mock_model_class.expects(:find).returns({:foo => :bar})
-        @controller.find.should == { :foo => :bar }
-    end
-    
-    it "should delegate search to the indirection's model class's search" do
-        @mock_model_class.expects(:search).returns({:foo => :bar})
-        @controller.search.should == { :foo => :bar }
-    end
-    
-    it "should delegate destroy to the indirection's model class's destroy" do
-        @mock_model_class.expects(:destroy).returns({:foo => :bar})
-        @controller.destroy.should == { :foo => :bar }    
-    end
-    
-    it "should delegate save to the indirection's model class's save" do
-        data = { :bar => :xyzzy }
-        mock_model_instance = mock('model instance')
-        @mock_model_class.expects(:new).with(data).returns(mock_model_instance)
-        mock_model_instance.expects(:save).returns({:foo => :bar})
-        @controller.save(data).should == { :foo => :bar }            
-    end
-end
\ No newline at end of file
diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb
index 49562e866..c45ca8d66 100644
--- a/spec/unit/network/http/mongrel/rest.rb
+++ b/spec/unit/network/http/mongrel/rest.rb
@@ -49,12 +49,52 @@ describe Puppet::Network::HTTP::MongrelREST, "when initializing" do
 end
 
 describe Puppet::Network::HTTP::MongrelREST, "when receiving a request" do
+    before do
+        @mock_request = mock('mongrel http request')
+        @mock_response = mock('mongrel http response')
+        @mock_model_class = mock('indirected model class')
+        Puppet::Indirector::Indirection.stubs(:model).with(:foo).returns(@mock_model_class)
+        @mock_mongrel = mock('mongrel http server')
+        @mock_mongrel.stubs(:register)
+        @handler = Puppet::Network::HTTP::MongrelREST.new(:server => @mock_mongrel, :handler => :foo)
+    end
+    
+    it "should call the model find method if the request represents a singular HTTP GET" do
+        @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'GET', Mongrel::Const::REQUEST_PATH => '/foo'})
+        @mock_model_class.expects(:find)
+        @handler.process(@mock_request, @mock_response)
+    end
+
+    it "should call the model search method if the request represents a plural HTTP GET" do
+        @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'GET', Mongrel::Const::REQUEST_PATH => '/foos'})
+        @mock_model_class.expects(:search)
+        @handler.process(@mock_request, @mock_response)
+    end
+    
+    it "should call the model destroy method if the request represents an HTTP DELETE" do
+        @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'DELETE', Mongrel::Const::REQUEST_PATH => '/foo'})
+        @mock_model_class.expects(:destroy)
+        @handler.process(@mock_request, @mock_response)
+    end
+
+    it "should call the model save method if the request represents an HTTP PUT" do
+        @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'PUT', Mongrel::Const::REQUEST_PATH => '/foo'})
+        mock_model_instance = mock('indirected model instance')
+        mock_model_instance.expects(:save)
+        @mock_model_class.expects(:new).returns(mock_model_instance)
+        @handler.process(@mock_request, @mock_response)
+    end
+    
+    it "should fail if the HTTP method isn't supported" do
+        @mock_request.stubs(:params).returns({ Mongrel::Const::REQUEST_METHOD => 'POST', Mongrel::Const::REQUEST_PATH => '/foo'})
+        Proc.new { @handler.process(@mock_request, @mock_response) }.should raise_error(ArgumentError)
+    end
+
     it "should unpack request information from Mongrel"
+    
     it "should unpack parameters from the request for passing to controller methods"    
-    it "should call the controller find method if the request represents a singular HTTP GET"
-    it "should call the controller search method if the request represents a plural HTTP GET"
-    it "should call the controller destroy method if the request represents an HTTP DELETE"
-    it "should call the controller save method if the request represents an HTTP PUT"
+    
     it "should serialize the result from the controller method for return back to Mongrel"
-    it "should serialize a controller expection result for return back to Mongrel"
+    
+    it "should serialize a controller exception result for return back to Mongrel"
 end
diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb
index 70c50bf39..418c97b6f 100644
--- a/spec/unit/network/http/webrick/rest.rb
+++ b/spec/unit/network/http/webrick/rest.rb
@@ -50,11 +50,18 @@ end
 
 describe Puppet::Network::HTTP::WEBrickREST, "when receiving a request" do
     it "should unpack request information from WEBrick"
+    
     it "should unpack parameters from the request for passing to controller methods"    
+    
     it "should call the controller find method if the request represents a singular HTTP GET"
+    
     it "should call the controller search method if the request represents a plural HTTP GET"
+    
     it "should call the controller destroy method if the request represents an HTTP DELETE"
+    
     it "should call the controller save method if the request represents an HTTP PUT"
-    it "should serialize the result from the controller method for return back to Mongrel"
-    it "should serialize a controller expection result for return back to Mongrel"
+    
+    it "should serialize the result from the controller method for return back to WEBrick"
+    
+    it "should serialize a controller exception result for return back to WEBrick"
 end