refactoring out resource loading/building logic into separate class

lib/cancan.rb

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

lib/cancan/controller_resource.rb

@@ -0,0 +1,39 @@
+module CanCan
+  class ControllerResource # :nodoc:
+    def initialize(controller, name, parent = nil)
+      @controller = controller
+      @name = name
+      @parent = parent
+    end
+    
+    def model_class
+      @name.to_s.camelize.constantize
+    end
+    
+    def find(id)
+      self.model_instance = base.find(id)
+    end
+    
+    def build(attributes)
+      if base.kind_of? Class
+        self.model_instance = base.new(attributes)
+      else
+        self.model_instance = base.build(attributes)
+      end
+    end
+    
+    def model_instance
+      @controller.instance_variable_get("@#{@name}")
+    end
+    
+    def model_instance=(instance)
+      @controller.instance_variable_set("@#{@name}", instance)
+    end
+    
+    private
+    
+    def base
+      @parent ? @parent.model_instance.send(@name.to_s.pluralize) : model_class
+    end
+  end
+end

lib/cancan/resource_authorization.rb

@@ -14,66 +14,37 @@ module CanCan
     end
     
     def load_resource
-      load_parent if @options[:nested]
       unless collection_actions.include? params[:action].to_sym
         if new_actions.include? params[:action].to_sym
-          if parent_instance
+          resource.build(params[model_name.to_sym])
-            self.model_instance = parent_instance.send(model_name.pluralize).build(params[model_name.to_sym])
-          else
-            self.model_instance = model_class.new(params[model_name.to_sym])
-          end
         elsif params[:id]
-          if parent_instance
+          resource.find(params[:id])
-            self.model_instance = parent_instance.send(model_name.pluralize).find(params[:id])
-          else
-            self.model_instance = model_class.find(params[:id])
-          end
         end
       end
     end
     
     def authorize_resource
-      @controller.unauthorized! if @controller.cannot?(params[:action].to_sym, model_instance || model_class)
+      @controller.unauthorized! if @controller.cannot?(params[:action].to_sym, resource.model_instance || resource.model_class)
     end
     
     private
     
+    def resource
+      @resource ||= ControllerResource.new(@controller, model_name, parent_resource)
+    end
+    
+    def parent_resource
+      if @options[:nested]
+        parent = ControllerResource.new(@controller, @options[:nested])
+        parent.find(@params["#{@options[:nested]}_id".to_sym])
+        parent
+      end
+    end
+    
     def model_name
       params[:controller].split('/').last.singularize
     end
     
-    def model_class
-      model_name.camelcase.constantize
-    end
-    
-    def load_parent
-      self.parent_instance = parent_class.find(parent_id)
-    end
-    
-    def parent_class
-      @options[:nested].to_s.camelcase.constantize
-    end
-    
-    def parent_id
-      @params["#{@options[:nested]}_id".to_sym]
-    end
-    
-    def model_instance
-      @controller.instance_variable_get("@#{model_name}")
-    end
-    
-    def model_instance=(instance)
-      @controller.instance_variable_set("@#{model_name}", instance)
-    end
-    
-    def parent_instance
-      @controller.instance_variable_get("@#{@options[:nested]}")
-    end
-    
-    def parent_instance=(instance)
-      @controller.instance_variable_set("@#{@options[:nested]}", instance)
-    end
-    
     def collection_actions
       [:index] + [@options[:collection]].flatten
     end

spec/cancan/controller_resource_spec.rb

@@ -0,0 +1,37 @@
+require File.dirname(__FILE__) + '/../spec_helper'
+
+describe CanCan::ControllerResource do
+  before(:each) do
+    @controller = Object.new
+  end
+  
+  it "should determine model class by constantizing give name" do
+    CanCan::ControllerResource.new(@controller, :ability).model_class.should == Ability
+  end
+  
+  it "should fetch model through model class and assign it to the instance" do
+    stub(Ability).find(123) { :some_ability }
+    CanCan::ControllerResource.new(@controller, :ability).find(123)
+    @controller.instance_variable_get(:@ability).should == :some_ability
+  end
+  
+  it "should fetch model through parent and assign it to the instance" do
+    parent = Object.new
+    stub(parent).model_instance.stub!.abilities.stub!.find(123) { :some_ability }
+    CanCan::ControllerResource.new(@controller, :ability, parent).find(123)
+    @controller.instance_variable_get(:@ability).should == :some_ability
+  end
+  
+  it "should build model through model class and assign it to the instance" do
+    stub(Ability).new(123) { :some_ability }
+    CanCan::ControllerResource.new(@controller, :ability).build(123)
+    @controller.instance_variable_get(:@ability).should == :some_ability
+  end
+  
+  it "should build model through parent and assign it to the instance" do
+    parent = Object.new
+    stub(parent).model_instance.stub!.abilities.stub!.build(123) { :some_ability }
+    CanCan::ControllerResource.new(@controller, :ability, parent).build(123)
+    @controller.instance_variable_get(:@ability).should == :some_ability
+  end
+end