From 721939babd5e91b8d515473cbf7dd8d828dd2a7f Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Fri, 3 Sep 2010 14:00:46 -0700 Subject: [PATCH] cleaning up some internal specs and names --- lib/cancan/ability.rb | 2 +- lib/cancan/can_definition.rb | 2 +- spec/cancan/active_record_additions_spec.rb | 2 +- spec/cancan/can_definition_spec.rb | 1 + spec/cancan/controller_resource_spec.rb | 170 ++++++++++---------- spec/cancan/query_spec.rb | 96 +++++------ spec/spec_helper.rb | 14 +- 7 files changed, 145 insertions(+), 142 deletions(-) diff --git a/lib/cancan/ability.rb b/lib/cancan/ability.rb index 5b6d846..7b74fef 100644 --- a/lib/cancan/ability.rb +++ b/lib/cancan/ability.rb @@ -209,7 +209,7 @@ module CanCan def attributes_for(action, subject) attributes = {} relevant_can_definitions(action, subject).map do |can_definition| - attributes.merge!(can_definition.new_attributes) if can_definition.base_behavior + attributes.merge!(can_definition.attributes_from_conditions) if can_definition.base_behavior end attributes end diff --git a/lib/cancan/can_definition.rb b/lib/cancan/can_definition.rb index 41442df..14c6c8a 100644 --- a/lib/cancan/can_definition.rb +++ b/lib/cancan/can_definition.rb @@ -67,7 +67,7 @@ module CanCan hash end - def new_attributes + def attributes_from_conditions attributes = {} @conditions.each do |key, value| attributes[key] = value unless [Array, Range, Hash].include? value.class diff --git a/spec/cancan/active_record_additions_spec.rb b/spec/cancan/active_record_additions_spec.rb index 1675a4b..d9431d6 100644 --- a/spec/cancan/active_record_additions_spec.rb +++ b/spec/cancan/active_record_additions_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe CanCan::ActiveRecordAdditions do before(:each) do - @model_class = Class.new(Person) + @model_class = Class.new(Project) stub(@model_class).scoped { :scoped_stub } @model_class.send(:include, CanCan::ActiveRecordAdditions) @ability = Object.new diff --git a/spec/cancan/can_definition_spec.rb b/spec/cancan/can_definition_spec.rb index 2b2851a..dab6345 100644 --- a/spec/cancan/can_definition_spec.rb +++ b/spec/cancan/can_definition_spec.rb @@ -1,5 +1,6 @@ require "spec_helper" +# Most of CanDefinition functionality is tested in Ability specs describe CanCan::CanDefinition do before(:each) do @conditions = {} diff --git a/spec/cancan/controller_resource_spec.rb b/spec/cancan/controller_resource_spec.rb index b56dae5..4b30046 100644 --- a/spec/cancan/controller_resource_spec.rb +++ b/spec/cancan/controller_resource_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe CanCan::ControllerResource do before(:each) do - @params = HashWithIndifferentAccess.new(:controller => "abilities") + @params = HashWithIndifferentAccess.new(:controller => "projects") @controller = Object.new # simple stub for now stub(@controller).params { @params } stub(@controller).current_ability.stub!.attributes_for { {} } @@ -10,74 +10,74 @@ describe CanCan::ControllerResource do it "should load the resource into an instance variable if params[:id] is specified" do @params.merge!(:action => "show", :id => 123) - stub(Ability).find(123) { :some_resource } + stub(Project).find(123) { :some_project } resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should not load resource into an instance variable if already set" do @params.merge!(:action => "show", :id => 123) - @controller.instance_variable_set(:@ability, :some_ability) + @controller.instance_variable_set(:@project, :some_project) resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_ability + @controller.instance_variable_get(:@project).should == :some_project end it "should properly load resource for namespaced controller" do - @params.merge!(:controller => "admin/abilities", :action => "show", :id => 123) - stub(Ability).find(123) { :some_resource } + @params.merge!(:controller => "admin/projects", :action => "show", :id => 123) + stub(Project).find(123) { :some_project } resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should properly load resource for namespaced controller when using '::' for namespace" do - @params.merge!(:controller => "Admin::AbilitiesController", :action => "show", :id => 123) - stub(Ability).find(123) { :some_resource } + @params.merge!(:controller => "Admin::ProjectsController", :action => "show", :id => 123) + stub(Project).find(123) { :some_project } resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should build a new resource with hash if params[:id] is not specified" do - @params.merge!(:action => "create", :ability => {:foo => "bar"}) - stub(Ability).new("foo" => "bar") { :some_resource } + @params.merge!(:action => "create", :project => {:foo => "bar"}) + stub(Project).new("foo" => "bar") { :some_project } resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should build a new resource with attributes from current ability" do - @params[:controller] = "people" + @params[:controller] = "projects" @params[:action] = "new" - person = Object.new - mock(Person).new { person } - mock(person).name = "foobar" - stub(@controller).current_ability.stub!.attributes_for(:new, Person) { {:name => "foobar"} } + project = Object.new + mock(Project).new { project } + mock(project).name = "foobar" + stub(@controller).current_ability.stub!.attributes_for(:new, Project) { {:name => "foobar"} } resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@person).should == person + @controller.instance_variable_get(:@project).should == project end it "should not build a resource when on index action" do @params[:action] = "index" resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should be_nil + @controller.instance_variable_get(:@project).should be_nil end it "should perform authorization using controller action and loaded model" do @params[:action] = "show" - @controller.instance_variable_set(:@ability, :some_resource) - stub(@controller).authorize!(:show, :some_resource) { raise CanCan::AccessDenied } + @controller.instance_variable_set(:@project, :some_project) + stub(@controller).authorize!(:show, :some_project) { raise CanCan::AccessDenied } resource = CanCan::ControllerResource.new(@controller) lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied) end it "should perform authorization using controller action and non loaded model" do @params[:action] = "show" - stub(@controller).authorize!(:show, Ability) { raise CanCan::AccessDenied } + stub(@controller).authorize!(:show, Project) { raise CanCan::AccessDenied } resource = CanCan::ControllerResource.new(@controller) lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied) end @@ -94,155 +94,147 @@ describe CanCan::ControllerResource do @params[:action] = "sort" resource = CanCan::ControllerResource.new(@controller, :collection => [:sort, :list]) resource.load_resource - @controller.instance_variable_get(:@ability).should be_nil + @controller.instance_variable_get(:@project).should be_nil end it "should build a resource when on custom new action even when params[:id] exists" do @params.merge!(:action => "build", :id => 123) - stub(Ability).new { :some_resource } + stub(Project).new { :some_project } resource = CanCan::ControllerResource.new(@controller, :new => :build) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should not try to load resource for other action if params[:id] is undefined" do @params[:action] = "list" resource = CanCan::ControllerResource.new(@controller) resource.load_resource - @controller.instance_variable_get(:@ability).should be_nil + @controller.instance_variable_get(:@project).should be_nil end it "should be a parent resource when name is provided which doesn't match controller" do - resource = CanCan::ControllerResource.new(@controller, :person) + resource = CanCan::ControllerResource.new(@controller, :category) resource.should be_parent end it "should not be a parent resource when name is provided which matches controller" do - resource = CanCan::ControllerResource.new(@controller, :ability) + resource = CanCan::ControllerResource.new(@controller, :project) resource.should_not be_parent end it "should be parent if specified in options" do - resource = CanCan::ControllerResource.new(@controller, :ability, {:parent => true}) + resource = CanCan::ControllerResource.new(@controller, :project, {:parent => true}) resource.should be_parent end it "should not be parent if specified in options" do - resource = CanCan::ControllerResource.new(@controller, :person, {:parent => false}) + resource = CanCan::ControllerResource.new(@controller, :category, {:parent => false}) resource.should_not be_parent end - it "should load parent resource through proper id parameter when supplying a resource with a different name" do - @params.merge!(:action => "index", :person_id => 123) - stub(Person).find(123) { :some_person } - resource = CanCan::ControllerResource.new(@controller, :person) + it "should load parent resource through proper id parameter" do + @params.merge!(:action => "index", :project_id => 123) + stub(Project).find(123) { :some_project } + resource = CanCan::ControllerResource.new(@controller, :project, :parent => true) resource.load_resource - @controller.instance_variable_get(:@person).should == :some_person - end - - it "should load parent resource for collection action" do - @params.merge!(:action => "index", :person_id => 456) - stub(Person).find(456) { :some_person } - resource = CanCan::ControllerResource.new(@controller, :person) - resource.load_resource - @controller.instance_variable_get(:@person).should == :some_person + @controller.instance_variable_get(:@project).should == :some_project end it "should load resource through the association of another parent resource" do @params.merge!(:action => "show", :id => 123) - person = Object.new - @controller.instance_variable_set(:@person, person) - stub(person).abilities.stub!.find(123) { :some_ability } - resource = CanCan::ControllerResource.new(@controller, :through => :person) + category = Object.new + @controller.instance_variable_set(:@category, category) + stub(category).projects.stub!.find(123) { :some_project } + resource = CanCan::ControllerResource.new(@controller, :through => :category) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_ability + @controller.instance_variable_get(:@project).should == :some_project end it "should not load through parent resource if instance isn't loaded" do @params.merge!(:action => "show", :id => 123) - stub(Ability).find(123) { :some_ability } - resource = CanCan::ControllerResource.new(@controller, :through => :person) + stub(Project).find(123) { :some_project } + resource = CanCan::ControllerResource.new(@controller, :through => :category) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_ability + @controller.instance_variable_get(:@project).should == :some_project end it "should authorize nested resource through parent association on index action" do @params.merge!(:action => "index") - person = Object.new - @controller.instance_variable_set(:@person, person) - stub(@controller).authorize!(:index, person => Ability) { raise CanCan::AccessDenied } - resource = CanCan::ControllerResource.new(@controller, :through => :person) + category = Object.new + @controller.instance_variable_set(:@category, category) + stub(@controller).authorize!(:index, category => Project) { raise CanCan::AccessDenied } + resource = CanCan::ControllerResource.new(@controller, :through => :category) lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied) end it "should load through first matching if multiple are given" do @params.merge!(:action => "show", :id => 123) - person = Object.new - @controller.instance_variable_set(:@person, person) - stub(person).abilities.stub!.find(123) { :some_ability } - resource = CanCan::ControllerResource.new(@controller, :through => [:thing, :person]) + category = Object.new + @controller.instance_variable_set(:@category, category) + stub(category).projects.stub!.find(123) { :some_project } + resource = CanCan::ControllerResource.new(@controller, :through => [:category, :user]) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_ability + @controller.instance_variable_get(:@project).should == :some_project end it "should find record through has_one association with :singleton option" do @params.merge!(:action => "show") - person = Object.new - @controller.instance_variable_set(:@person, person) - stub(person).ability { :some_ability } - resource = CanCan::ControllerResource.new(@controller, :through => :person, :singleton => true) + category = Object.new + @controller.instance_variable_set(:@category, category) + stub(category).project { :some_project } + resource = CanCan::ControllerResource.new(@controller, :through => :category, :singleton => true) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_ability + @controller.instance_variable_get(:@project).should == :some_project end it "should build record through has_one association with :singleton option" do - @params.merge!(:action => "create", :ability => :ability_attributes) - person = Object.new - @controller.instance_variable_set(:@person, person) - stub(person).build_ability(:ability_attributes) { :new_ability } - resource = CanCan::ControllerResource.new(@controller, :through => :person, :singleton => true) + @params.merge!(:action => "create", :project => :project_attributes) + category = Object.new + @controller.instance_variable_set(:@category, category) + stub(category).build_project(:project_attributes) { :new_project } + resource = CanCan::ControllerResource.new(@controller, :through => :category, :singleton => true) resource.load_resource - @controller.instance_variable_get(:@ability).should == :new_ability + @controller.instance_variable_get(:@project).should == :new_project end it "should only authorize :read action on parent resource" do - @params.merge!(:action => "new", :person_id => 123) - stub(Person).find(123) { :some_person } - stub(@controller).authorize!(:read, :some_person) { raise CanCan::AccessDenied } - resource = CanCan::ControllerResource.new(@controller, :person) + @params.merge!(:action => "new", :project_id => 123) + stub(Project).find(123) { :some_project } + stub(@controller).authorize!(:read, :some_project) { raise CanCan::AccessDenied } + resource = CanCan::ControllerResource.new(@controller, :project, :parent => true) lambda { resource.load_and_authorize_resource }.should raise_error(CanCan::AccessDenied) end it "should load the model using a custom class" do @params.merge!(:action => "show", :id => 123) - stub(Person).find(123) { :some_resource } - resource = CanCan::ControllerResource.new(@controller, :class => Person) + stub(Project).find(123) { :some_project } + resource = CanCan::ControllerResource.new(@controller, :class => Project) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should authorize based on resource name if class is false" do @params.merge!(:action => "show", :id => 123) - stub(@controller).authorize!(:show, :ability) { raise CanCan::AccessDenied } + stub(@controller).authorize!(:show, :project) { raise CanCan::AccessDenied } resource = CanCan::ControllerResource.new(@controller, :class => false) lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied) end it "should load and authorize using custom instance name" do @params.merge!(:action => "show", :id => 123) - stub(Ability).find(123) { :some_ability } - stub(@controller).authorize!(:show, :some_ability) { raise CanCan::AccessDenied } - resource = CanCan::ControllerResource.new(@controller, :instance_name => :custom_ability) + stub(Project).find(123) { :some_project } + stub(@controller).authorize!(:show, :some_project) { raise CanCan::AccessDenied } + resource = CanCan::ControllerResource.new(@controller, :instance_name => :custom_project) lambda { resource.load_and_authorize_resource }.should raise_error(CanCan::AccessDenied) - @controller.instance_variable_get(:@custom_ability).should == :some_ability + @controller.instance_variable_get(:@custom_project).should == :some_project end it "should load resource using custom find_by attribute" do @params.merge!(:action => "show", :id => 123) - stub(Ability).find_by_name!(123) { :some_resource } + stub(Project).find_by_name!(123) { :some_project } resource = CanCan::ControllerResource.new(@controller, :find_by => :name) resource.load_resource - @controller.instance_variable_get(:@ability).should == :some_resource + @controller.instance_variable_get(:@project).should == :some_project end it "should raise ImplementationRemoved when adding :name option" do @@ -253,13 +245,13 @@ describe CanCan::ControllerResource do it "should raise ImplementationRemoved exception when specifying :resource option since it is no longer used" do lambda { - CanCan::ControllerResource.new(@controller, :resource => Person) + CanCan::ControllerResource.new(@controller, :resource => Project) }.should raise_error(CanCan::ImplementationRemoved) end it "should raise ImplementationRemoved exception when passing :nested option" do lambda { - CanCan::ControllerResource.new(@controller, :nested => :person) + CanCan::ControllerResource.new(@controller, :nested => :project) }.should raise_error(CanCan::ImplementationRemoved) end end diff --git a/spec/cancan/query_spec.rb b/spec/cancan/query_spec.rb index 63e3d96..a44b6a6 100644 --- a/spec/cancan/query_spec.rb +++ b/spec/cancan/query_spec.rb @@ -7,101 +7,101 @@ describe CanCan::Query do end it "should have false conditions if no abilities match" do - @ability.query(:destroy, Person).conditions.should == "true=false" + @ability.query(:destroy, Project).conditions.should == "true=false" end it "should return hash for single `can` definition" do - @ability.can :read, Person, :blocked => false, :user_id => 1 - @ability.query(:read, Person).conditions.should == { :blocked => false, :user_id => 1 } + @ability.can :read, Project, :blocked => false, :user_id => 1 + @ability.query(:read, Project).conditions.should == { :blocked => false, :user_id => 1 } end it "should merge multiple can definitions into single SQL string joining with OR" do - @ability.can :read, Person, :blocked => false - @ability.can :read, Person, :admin => true - @ability.query(:read, Person).conditions.should == "(admin=true) OR (blocked=false)" + @ability.can :read, Project, :blocked => false + @ability.can :read, Project, :admin => true + @ability.query(:read, Project).conditions.should == "(admin=true) OR (blocked=false)" end it "should merge multiple can definitions into single SQL string joining with OR and AND" do - @ability.can :read, Person, :blocked => false, :active => true - @ability.can :read, Person, :admin => true - @ability.query(:read, Person).conditions.should orderlessly_match("(blocked=false AND active=true) OR (admin=true)") + @ability.can :read, Project, :blocked => false, :active => true + @ability.can :read, Project, :admin => true + @ability.query(:read, Project).conditions.should orderlessly_match("(blocked=false AND active=true) OR (admin=true)") end it "should merge multiple can definitions into single SQL string joining with OR and AND" do - @ability.can :read, Person, :blocked => false, :active => true - @ability.can :read, Person, :admin => true - @ability.query(:read, Person).conditions.should orderlessly_match("(blocked=false AND active=true) OR (admin=true)") + @ability.can :read, Project, :blocked => false, :active => true + @ability.can :read, Project, :admin => true + @ability.query(:read, Project).conditions.should orderlessly_match("(blocked=false AND active=true) OR (admin=true)") end it "should return false conditions for cannot clause" do - @ability.cannot :read, Person - @ability.query(:read, Person).conditions.should == "true=false" + @ability.cannot :read, Project + @ability.query(:read, Project).conditions.should == "true=false" end it "should return SQL for single `can` definition in front of default `cannot` condition" do - @ability.cannot :read, Person - @ability.can :read, Person, :blocked => false, :user_id => 1 - @ability.query(:read, Person).conditions.should orderlessly_match("blocked=false AND user_id=1") + @ability.cannot :read, Project + @ability.can :read, Project, :blocked => false, :user_id => 1 + @ability.query(:read, Project).conditions.should orderlessly_match("blocked=false AND user_id=1") end it "should return true condition for single `can` definition in front of default `can` condition" do - @ability.can :read, Person - @ability.can :read, Person, :blocked => false, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=true' + @ability.can :read, Project + @ability.can :read, Project, :blocked => false, :user_id => 1 + @ability.query(:read, Project).conditions.should == 'true=true' end it "should return false condition for single `cannot` definition" do - @ability.cannot :read, Person, :blocked => true, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=false' + @ability.cannot :read, Project, :blocked => true, :user_id => 1 + @ability.query(:read, Project).conditions.should == 'true=false' end it "should return `false condition` for single `cannot` definition in front of default `cannot` condition" do - @ability.cannot :read, Person - @ability.cannot :read, Person, :blocked => true, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=false' + @ability.cannot :read, Project + @ability.cannot :read, Project, :blocked => true, :user_id => 1 + @ability.query(:read, Project).conditions.should == 'true=false' end it "should return `not (sql)` for single `cannot` definition in front of default `can` condition" do - @ability.can :read, Person - @ability.cannot :read, Person, :blocked => true, :user_id => 1 - @ability.query(:read, Person).conditions.should orderlessly_match("not (blocked=true AND user_id=1)") + @ability.can :read, Project + @ability.cannot :read, Project, :blocked => true, :user_id => 1 + @ability.query(:read, Project).conditions.should orderlessly_match("not (blocked=true AND user_id=1)") end it "should return appropriate sql conditions in complex case" do - @ability.can :read, Person - @ability.can :manage, Person, :id => 1 - @ability.can :update, Person, :manager_id => 1 - @ability.cannot :update, Person, :self_managed => true - @ability.query(:update, Person).conditions.should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' - @ability.query(:manage, Person).conditions.should == {:id=>1} - @ability.query(:read, Person).conditions.should == 'true=true' + @ability.can :read, Project + @ability.can :manage, Project, :id => 1 + @ability.can :update, Project, :manager_id => 1 + @ability.cannot :update, Project, :self_managed => true + @ability.query(:update, Project).conditions.should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' + @ability.query(:manage, Project).conditions.should == {:id=>1} + @ability.query(:read, Project).conditions.should == 'true=true' end it "should have nil joins if no can definitions" do - @ability.query(:read, Person).joins.should be_nil + @ability.query(:read, Project).joins.should be_nil end it "should have nil joins if no nested hashes specified in conditions" do - @ability.can :read, Person, :blocked => false - @ability.can :read, Person, :admin => true - @ability.query(:read, Person).joins.should be_nil + @ability.can :read, Project, :blocked => false + @ability.can :read, Project, :admin => true + @ability.query(:read, Project).joins.should be_nil end it "should merge separate joins into a single array" do - @ability.can :read, Person, :project => { :blocked => false } - @ability.can :read, Person, :company => { :admin => true } - @ability.query(:read, Person).joins.inspect.should orderlessly_match([:company, :project].inspect) + @ability.can :read, Project, :project => { :blocked => false } + @ability.can :read, Project, :company => { :admin => true } + @ability.query(:read, Project).joins.inspect.should orderlessly_match([:company, :project].inspect) end it "should merge same joins into a single array" do - @ability.can :read, Person, :project => { :blocked => false } - @ability.can :read, Person, :project => { :admin => true } - @ability.query(:read, Person).joins.should == [:project] + @ability.can :read, Project, :project => { :blocked => false } + @ability.can :read, Project, :project => { :admin => true } + @ability.query(:read, Project).joins.should == [:project] end it "should merge complex, nested joins" do - @ability.can :read, Person, :project => { :bar => {:test => true} }, :company => { :bar => {:test => true} } - @ability.can :read, Person, :project => { :foo => {:bar => true}, :bar => {:zip => :zap} } - @ability.query(:read, Person).joins.inspect.should orderlessly_match([{:project => [:bar, :foo]}, {:company => [:bar]}].inspect) + @ability.can :read, Project, :project => { :bar => {:test => true} }, :company => { :bar => {:test => true} } + @ability.can :read, Project, :project => { :foo => {:bar => true}, :bar => {:zip => :zap} } + @ability.query(:read, Project).joins.inspect.should orderlessly_match([{:project => [:bar, :foo]}, {:company => [:bar]}].inspect) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2ed5bc3..19f5eaa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,8 +19,18 @@ class Ability end end -# this class helps out in testing SQL conditions -class Person +# Generic class to mimic a model +class Project + attr_accessor :name + + def initialize(attributes = {}) + @name = attributes[:name] + end + + def attributes=(attributes) + @name = attributes[:name] if attributes[:name] + end + class << self protected