adding :through option to replace :nesting option and moving Resource…

…Authorization class code into ControllerResource

CHANGELOG.rdoc

@@ -1,3 +1,9 @@
+* Changing :resource option in load/authorize_resource back to :class with ability to pass false
+
+* Removing :nested option in favor of :through option with separate load/authorize call
+
+* Moving internal logic from ResourceAuthorization to ControllerResource class
+
 * Supporting multiple "can" and "cannot" calls with accessible_by (thanks funny-falcon) - see issue #71
 
 * Supporting deeply nested aliases - see issue #98

lib/cancan.rb

@@ -1,7 +1,6 @@
 require 'cancan/ability'
 require 'cancan/can_definition'
 require 'cancan/controller_resource'
-require 'cancan/resource_authorization'
 require 'cancan/controller_additions'
 require 'cancan/active_record_additions'
 require 'cancan/exceptions'

lib/cancan/controller_additions.rb

@@ -11,11 +11,11 @@ module ClassMethods
       #     load_and_authorize_resource
       #   end
       #
-      def load_and_authorize_resource(options = {})
-        ResourceAuthorization.add_before_filter(self, :load_and_authorize_resource, options)
+      def load_and_authorize_resource(*args)
+        ControllerResource.add_before_filter(self, :load_and_authorize_resource, *args)
       end
 
-      # Sets up a before filter which loads the appropriate model resource into an instance variable.
+      # Sets up a before filter which loads the model resource into an instance variable.
       # For example, given an ArticlesController it will load the current article into the @article
       # instance variable. It does this by either calling Article.find(params[:id]) or
       # Article.new(params[:article]) depending upon the action. It does nothing for the "index"
@@ -41,6 +41,20 @@ def load_and_authorize_resource(options = {})
       #     end
       #   end
       #
+      # If a name is provided which does not match the controller it assumes it is a parent resource. Child
+      # resources can then be loaded through it.
+      #
+      #   class BooksController < ApplicationController
+      #     load_resource :author
+      #     load_resource :book, :through => :author
+      #   end
+      #
+      # Here the author resource will be loaded before each action using params[:author_id]. The book resource
+      # will then be loaded through the @author instance variable.
+      #
+      # That first argument is optional and will default to the singular name of the controller.
+      # A hash of options (see below) can also be passed to this method to further customize it.
+      #
       # See load_and_authorize_resource to automatically authorize the resource too.
       #
       # Options:
@@ -50,27 +64,22 @@ def load_and_authorize_resource(options = {})
       # [:+except+]
       #   Does not apply before filter to given actions.
       #
-      # [:+nested+]
-      #   Specify which resource this is nested under.
-      #
-      #     load_resource :nested => :author
-      #
-      #   Deep nesting can be defined in an array.
-      #
-      #     load_resource :nested => [:publisher, :author]
-      #
-      # [:+name+]
-      #   The name of the resource if it cannot be determined from controller (string or symbol).
+      # [:+through+]
+      #   Load this resource through another one. This should match the name of the parent instance variable.
       #
-      #     load_resource :name => :article
+      # [:+parent+]
+      #   True or false depending on if the resource is considered a parent resource. This defaults to +true+ if a resource
+      #   name is given which does not match the controller.
       #
-      # [:+resource+]
+      # [:+class+]
       #   The class to use for the model (string or constant).
       #
+      # [:+instance_name+]
+      #   The name of the instance variable to load the resource into.
+      #
       # [:+collection+]
       #   Specify which actions are resource collection actions in addition to :+index+. This
-      #   is usually not necessary because it will try to guess depending on if an :+id+
-      #   is present in +params+.
+      #   is usually not necessary because it will try to guess depending on if the id param is present.
       #
       #     load_resource :collection => [:sort, :list]
       #
@@ -81,11 +90,11 @@ def load_and_authorize_resource(options = {})
       #
       #     load_resource :new => :build
       #
-      def load_resource(options = {})
-        ResourceAuthorization.add_before_filter(self, :load_resource, options)
+      def load_resource(*args)
+        ControllerResource.add_before_filter(self, :load_resource, *args)
       end
 
-      # Sets up a before filter which authorizes the current resource using the instance variable.
+      # Sets up a before filter which authorizes the resource using the instance variable.
       # For example, if you have an ArticlesController it will check the @article instance variable
       # and ensure the user can perform the current action on it. Under the hood it is doing
       # something like the following.
@@ -98,6 +107,19 @@ def load_resource(options = {})
       #     authorize_resource
       #   end
       #
+      # If you pass in the name of a resource which does not match the controller it will assume
+      # it is a parent resource.
+      #
+      #   class BooksController < ApplicationController
+      #     authorize_resource :author
+      #     authorize_resource :book
+      #   end
+      #
+      # Here it will authorize :+show+, @+author+ on every action before authorizing the book.
+      #
+      # That first argument is optional and will default to the singular name of the controller.
+      # A hash of options (see below) can also be passed to this method to further customize it.
+      #
       # See load_and_authorize_resource to automatically load the resource too.
       #
       # Options:
@@ -107,17 +129,23 @@ def load_resource(options = {})
       # [:+except+]
       #   Does not apply before filter to given actions.
       #
-      # [:+name+]
-      #   The name of the resource if it cannot be determined from controller (string or symbol).
+      # [:+parent+]
+      #   True or false depending on if the resource is considered a parent resource. This defaults to +true+ if a resource
+      #   name is given which does not match the controller.
+      #
+      # [:+class+]
+      #   The class to use for the model (string or constant). This passed in when the instance variable is not set.
+      #   Pass +false+ if there is no associated class for this resource and it will use a symbol of the resource name.
       #
-      #     load_resource :name => :article
+      # [:+instance_name+]
+      #   The name of the instance variable for this resource.
       #
       # [:+resource+]
       #   The class to use for the model (string or constant). Alternatively pass a symbol
       #   to represent a resource which does not have a class.
       #
-      def authorize_resource(options = {})
-        ResourceAuthorization.add_before_filter(self, :authorize_resource, options)
+      def authorize_resource(*args)
+        ControllerResource.add_before_filter(self, :authorize_resource, *args)
       end
     end
 

lib/cancan/controller_resource.rb

@@ -1,54 +1,105 @@
 module CanCan
-  # Used internally to load and authorize a given controller resource.
-  # This manages finding or building an instance of the resource. If a
-  # parent is given it will go through the association.
+  # Handle the load and authorization controller logic so we don't clutter up all controllers with non-interface methods.
+  # This class is used internally, so you do not need to call methods directly on it.
   class ControllerResource # :nodoc:
-    def initialize(controller, name, parent = nil, options = {})
-      raise ImplementationRemoved, "The :class option has been renamed to :resource for specifying the class in CanCan." if options.has_key? :class
-      @controller = controller
-      @name = name
-      @parent = parent
-      @options = options
-    end
-
-    # Returns the class used for this resource. This can be overriden by the :resource option.
-    # Sometimes one will use a symbol as the resource if a class does not exist for it. In that
-    # case "find" and "build" should not be called on it.
-    def model_class
-      resource_class = @options[:resource]
-      if resource_class.nil?
-        @name.to_s.camelize.constantize
-      elsif resource_class.kind_of? String
-        resource_class.constantize
-      else
-        resource_class # could be a symbol
+    def self.add_before_filter(controller_class, method, options = {})
+      controller_class.before_filter(options.slice(:only, :except)) do |controller|
+        ControllerResource.new(controller, controller.params, options.except(:only, :except)).send(method)
       end
     end
 
-    def find(id)
-      self.model_instance ||= base.find(id)
+    def initialize(controller, params, *args)
+      @controller = controller
+      @params = params
+      @options = args.extract_options!
+      @name = args.first
+      raise CanCan::ImplementationRemoved, "The :nested option is no longer supported, instead use :through with separate load/authorize call." if @options[:nested]
+      raise CanCan::ImplementationRemoved, "The :name option is no longer supported, instead pass the name as the first argument." if @options[:name]
+      raise CanCan::ImplementationRemoved, "The :resource option has been renamed back to :class, use false if no class." if @options[:resource]
+    end
+
+    def load_and_authorize_resource
+      load_resource
+      authorize_resource
     end
 
-    # Build a new instance of this resource. If it is a class we just call "new" otherwise
-    # it's an associaiton and "build" is used.
-    def build(attributes)
-      self.model_instance ||= (base.kind_of?(Class) ? base.new(attributes) : base.build(attributes))
+    def load_resource
+      if !resource_instance && (parent? || member_action?)
+        @controller.instance_variable_set("@#{name}", load_resource_instance)
+      end
     end
 
-    def model_instance
-      @controller.instance_variable_get("@#{@name}")
+    def authorize_resource
+      @controller.authorize!(@params[:action].to_sym, resource_instance || resource_class)
     end
 
-    def model_instance=(instance)
-      @controller.instance_variable_set("@#{@name}", instance)
+    def parent?
+      @options[:parent] || @name && @name != name_from_controller.to_sym
     end
 
     private
 
+    def load_resource_instance
+      if !parent? && new_actions.include?(@params[:action].to_sym)
+        resource_base.kind_of?(Class) ? resource_base.new(attributes) : resource_base.build(attributes)
+      elsif id_param
+        resource_base.find(id_param)
+      end
+    end
+
+    def attributes
+      @params[name.to_sym]
+    end
+
+    def id_param
+      @params[parent? ? :"#{name}_id" : :id]
+    end
+
+    def member_action?
+      !collection_actions.include? @params[:action].to_sym
+    end
+
+    # Returns the class used for this resource. This can be overriden by the :class option.
+    # If +false+ is passed in it will use the resource name as a symbol in which case it should
+    # only be used for authorization, not loading since there's no class to load through.
+    def resource_class
+      case @options[:class]
+      when false  then name.to_sym
+      when nil    then name.to_s.camelize.constantize
+      when String then @options[:class].constantize
+      else @options[:class]
+      end
+    end
+
+    def resource_instance
+      @controller.instance_variable_get("@#{name}")
+    end
+
     # The object that methods (such as "find", "new" or "build") are called on.
-    # If there is a parent it will be the association, otherwise it will be the model's class.
-    def base
-      @parent ? @parent.model_instance.send(@name.to_s.pluralize) : model_class
+    # If the :through option is passed it will go through an association on that instance.
+    def resource_base
+      through_resource ? through_resource.send(name.to_s.pluralize) : resource_class
+    end
+
+    # The object to load this resource through.
+    def through_resource
+      @options[:through] && @controller.instance_variable_get("@#{@options[:through]}")
+    end
+
+    def name
+      @name || name_from_controller
+    end
+
+    def name_from_controller
+      @params[:controller].sub("Controller", "").underscore.split('/').last.singularize
+    end
+
+    def collection_actions
+      [:index] + [@options[:collection]].flatten
+    end
+
+    def new_actions
+      [:new, :create] + [@options[:new]].flatten
     end
   end
 end

spec/cancan/controller_additions_spec.rb

@@ -52,20 +52,20 @@
     @controller.cannot?(:foo, :bar).should be_true
   end
 
-  it "load_and_authorize_resource should setup a before filter which passes call to ResourceAuthorization" do
-    stub(CanCan::ResourceAuthorization).new(@controller, @controller.params, :foo => :bar).mock!.load_and_authorize_resource
+  it "load_and_authorize_resource should setup a before filter which passes call to ControllerResource" do
+    stub(CanCan::ControllerResource).new(@controller, @controller.params, :foo => :bar).mock!.load_and_authorize_resource
     mock(@controller_class).before_filter({}) { |options, block| block.call(@controller) }
     @controller_class.load_and_authorize_resource :foo => :bar
   end
 
-  it "authorize_resource should setup a before filter which passes call to ResourceAuthorization" do
-    stub(CanCan::ResourceAuthorization).new(@controller, @controller.params, :foo => :bar).mock!.authorize_resource
+  it "authorize_resource should setup a before filter which passes call to ControllerResource" do
+    stub(CanCan::ControllerResource).new(@controller, @controller.params, :foo => :bar).mock!.authorize_resource
     mock(@controller_class).before_filter(:except => :show) { |options, block| block.call(@controller) }
     @controller_class.authorize_resource :foo => :bar, :except => :show
   end
 
-  it "load_resource should setup a before filter which passes call to ResourceAuthorization" do
-    stub(CanCan::ResourceAuthorization).new(@controller, @controller.params, :foo => :bar).mock!.load_resource
+  it "load_resource should setup a before filter which passes call to ControllerResource" do
+    stub(CanCan::ControllerResource).new(@controller, @controller.params, :foo => :bar).mock!.load_resource
     mock(@controller_class).before_filter(:only => [:show, :index]) { |options, block| block.call(@controller) }
     @controller_class.load_resource :foo => :bar, :only => [:show, :index]
   end