refactoring out controller logic into separate ResourceAuthorization class - closes #11

lib/cancan.rb

@@ -5,4 +5,5 @@ module CanCan
 end
 
 require File.dirname(__FILE__) + '/cancan/ability'
+require File.dirname(__FILE__) + '/cancan/resource_authorization'
 require File.dirname(__FILE__) + '/cancan/controller_additions'

lib/cancan/controller_additions.rb

@@ -83,7 +83,7 @@ module CanCan
     #   before_filter :load_resource
     # 
     def load_resource # TODO this could use some refactoring
-      self.model_instance = params[:id] ? model_class.find(params[:id]) : model_class.new(params[model_name.to_sym]) unless params[:action] == "index"
+      ResourceAuthorization.new(self, params).load_resource
     end
     
     # Authorizes the resource in the current instance variable. For example,
@@ -99,7 +99,7 @@ module CanCan
     # 
     # See load_and_authorize_resource to automatically load the resource too.
     def authorize_resource # TODO this could use some refactoring
-      unauthorized! if cannot?(params[:action].to_sym, model_instance || model_class)
+      ResourceAuthorization.new(self, params).authorize_resource
     end
     
     # Calls load_resource to load the current resource model into an instance variable. 
@@ -109,28 +109,8 @@ module CanCan
     #   before_filter :load_and_authorize_resource
     # 
     def load_and_authorize_resource
-      load_resource
+      ResourceAuthorization.new(self, params).load_and_authorize_resource
-      authorize_resource
     end
-    
-    private
-    
-    def model_name
-       params[:controller].split('/').last.singularize
-    end
-    
-    def model_class
-      model_name.camelcase.constantize
-    end
-    
-    def model_instance
-      instance_variable_get("@#{model_name}")
-    end
-    
-    def model_instance=(instance)
-      instance_variable_set("@#{model_name}", instance)
-    end
-    
   end
 end
 

lib/cancan/resource_authorization.rb

@@ -0,0 +1,41 @@
+module CanCan
+  class ResourceAuthorization # :nodoc:
+    attr_reader :params
+    
+    def initialize(controller, params)
+      @controller = controller
+      @params = params
+    end
+    
+    def load_and_authorize_resource
+      load_resource
+      authorize_resource
+    end
+    
+    def load_resource
+      self.model_instance = params[:id] ? model_class.find(params[:id]) : model_class.new(params[model_name.to_sym]) unless params[:action] == "index"
+    end
+    
+    def authorize_resource
+      @controller.unauthorized! if @controller.cannot?(params[:action].to_sym, model_instance || model_class)
+    end
+    
+    private
+    
+    def model_name
+      params[:controller].split('/').last.singularize
+    end
+    
+    def model_class
+      model_name.camelcase.constantize
+    end
+    
+    def model_instance
+      @controller.instance_variable_get("@#{model_name}")
+    end
+    
+    def model_instance=(instance)
+      @controller.instance_variable_set("@#{model_name}", instance)
+    end
+  end
+end

spec/cancan/controller_additions_spec.rb

@@ -1,16 +1,10 @@
 require File.dirname(__FILE__) + '/../spec_helper'
 
-class Ability
-  include CanCan::Ability
-  
-  def initialize(user)
-  end
-end
-
 describe CanCan::ControllerAdditions do
   before(:each) do
     @controller_class = Class.new
     @controller = @controller_class.new
+    stub(@controller).params { {} }
     mock(@controller_class).helper_method(:can?, :cannot?)
     @controller_class.send(:include, CanCan::ControllerAdditions)
   end
@@ -33,62 +27,18 @@ describe CanCan::ControllerAdditions do
     @controller.cannot?(:foo, :bar).should be_true
   end
   
-  it "should load the resource if params[:id] is specified" do
+  it "should load resource" do
-    stub(@controller).params { {:controller => "abilities", :action => "show", :id => 123} }
+    mock.instance_of(CanCan::ResourceAuthorization).load_resource
-    stub(Ability).find(123) { :some_resource }
     @controller.load_resource
-    @controller.instance_variable_get(:@ability).should == :some_resource
   end
   
-  it "should build a new resource with hash if params[:id] is not specified" do
+  it "should authorize resource" do
-    stub(@controller).params { {:controller => "abilities", :action => "create", :ability => {:foo => "bar"}} }
+    mock.instance_of(CanCan::ResourceAuthorization).authorize_resource
-    stub(Ability).new(:foo => "bar") { :some_resource }
-    @controller.load_resource
-    @controller.instance_variable_get(:@ability).should == :some_resource
-  end
-  
-  it "should build a new resource even if attribute hash isn't specified" do
-    stub(@controller).params { {:controller => "abilities", :action => "new"} }
-    stub(Ability).new(nil) { :some_resource }
-    @controller.load_resource
-    @controller.instance_variable_get(:@ability).should == :some_resource
-  end
-  
-  it "should not build a resource when on index action" do
-    stub(@controller).params { {:controller => "abilities", :action => "index"} }
-    @controller.load_resource
-    @controller.instance_variable_get(:@ability).should be_nil
-  end
-  
-  it "should perform authorization using controller action and loaded model" do
-    stub(@controller).current_user { :current_user }
-    @controller.instance_variable_set(:@ability, :some_resource)
-    stub(@controller).params { {:controller => "abilities", :action => "show"} }
-    stub(@controller).can?(:show, :some_resource) { false }
-    lambda {
     @controller.authorize_resource
-    }.should raise_error(CanCan::AccessDenied)
-  end
-  
-  it "should perform authorization using controller action and non loaded model" do
-    stub(@controller).current_user { :current_user }
-    stub(@controller).params { {:controller => "abilities", :action => "show"} }
-    stub(@controller).can?(:show, Ability) { false }
-    lambda {
-      @controller.authorize_resource
-    }.should raise_error(CanCan::AccessDenied)
   end
   
   it "should load and authorize resource in one call" do
-    mock(@controller).load_resource
+    mock.instance_of(CanCan::ResourceAuthorization).load_and_authorize_resource
-    stub(@controller).authorize_resource
     @controller.load_and_authorize_resource
   end
-  
-  it "should properly load resource for namespaced controller" do
-    stub(@controller).params { {:controller => "admin/abilities", :action => "show", :id => 123} }
-    stub(Ability).find(123) { :some_resource }
-    @controller.load_resource
-    @controller.instance_variable_get(:@ability).should == :some_resource
-  end
 end

spec/cancan/resource_authorization_spec.rb

@@ -0,0 +1,59 @@
+require File.dirname(__FILE__) + '/../spec_helper'
+
+describe CanCan::ResourceAuthorization do
+  before(:each) do
+    @controller = Object.new # simple stub for now
+    stub(@controller).unauthorized! { raise CanCan::AccessDenied }
+  end
+  
+  it "should load the resource into an instance variable if params[:id] is specified" do
+    stub(Ability).find(123) { :some_resource }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "show", :id => 123)
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should == :some_resource
+  end
+  
+  it "should properly load resource for namespaced controller" do
+    stub(Ability).find(123) { :some_resource }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "admin/abilities", :action => "show", :id => 123)
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should == :some_resource
+  end
+  
+  it "should build a new resource with hash if params[:id] is not specified" do
+    stub(Ability).new(:foo => "bar") { :some_resource }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "create", :ability => {:foo => "bar"})
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should == :some_resource
+  end
+  
+  it "should build a new resource even if attribute hash isn't specified" do
+    stub(Ability).new(nil) { :some_resource }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "new")
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should == :some_resource
+  end
+  
+  it "should not build a resource when on index action" do
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "index")
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should be_nil
+  end
+  
+  it "should perform authorization using controller action and loaded model" do
+    @controller.instance_variable_set(:@ability, :some_resource)
+    stub(@controller).cannot?(:show, :some_resource) { true }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "show")
+    lambda {
+      authorization.authorize_resource
+    }.should raise_error(CanCan::AccessDenied)
+  end
+  
+  it "should perform authorization using controller action and non loaded model" do
+    stub(@controller).cannot?(:show, Ability) { true }
+    authorization = CanCan::ResourceAuthorization.new(@controller, :controller => "abilities", :action => "show")
+    lambda {
+      authorization.authorize_resource
+    }.should raise_error(CanCan::AccessDenied)
+  end
+end

spec/spec_helper.rb

@@ -9,3 +9,10 @@ require File.dirname(__FILE__) + '/../lib/cancan.rb'
 Spec::Runner.configure do |config|
   config.mock_with :rr
 end
+
+class Ability
+  include CanCan::Ability
+  
+  def initialize(user)
+  end
+end