From f6aaa581ef692404c662eae367063c42a552fd40 Mon Sep 17 00:00:00 2001 From: Tyler Gannon Date: Mon, 15 Nov 2010 19:43:54 -0800 Subject: [PATCH] can? should only go to db if there are mongoid criteria in the conditions. Easier to just do a simple comparison on the object in memory than to search the database. Also this allows method calls and other attributes that might not be found in the database. --- cancan.gemspec | 4 +-- lib/cancan/mongoid_additions.rb | 39 +++++++++++---------------- spec/cancan/mongoid_additions_spec.rb | 24 ++++++++++++++++- spec/spec_helper.rb | 4 ++- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/cancan.gemspec b/cancan.gemspec index 955bbe8..d5145c1 100644 --- a/cancan.gemspec +++ b/cancan.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |s| s.files = Dir["{lib,spec}/**/*", "[A-Z]*", "init.rb"] - ["Gemfile.lock"] s.require_path = "lib" - s.add_development_dependency 'rspec', '~> 2.0.0.beta.22' + s.add_development_dependency 'rspec', '~> 2.1.0' s.add_development_dependency 'rails', '~> 3.0.0' s.add_development_dependency 'rr', '~> 0.10.11' # 1.0.0 has respond_to? issues: http://github.com/btakita/rr/issues/issue/43 s.add_development_dependency 'supermodel', '~> 0.1.4' @@ -18,7 +18,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'mongoid', '~> 2.0.0.beta.19' s.add_development_dependency 'bson_ext', '~> 1.1' - s.add_development_dependency 'ruby-debug' + # s.add_development_dependency 'ruby-debug' s.rubyforge_project = s.name s.required_rubygems_version = ">= 1.3.4" diff --git a/lib/cancan/mongoid_additions.rb b/lib/cancan/mongoid_additions.rb index 5c05044..051f72b 100644 --- a/lib/cancan/mongoid_additions.rb +++ b/lib/cancan/mongoid_additions.rb @@ -36,37 +36,28 @@ module CanCan end # customize to handle Mongoid queries in ability definitions conditions + # Mongoid Criteria are simpler to check than normal conditions hashes + # When no conditions are given, true should be returned. + # The default CanCan behavior relies on the fact that conditions.all? will return true when conditions is empty + # The way ruby handles all? for empty hashes can be unexpected: + # {}.all?{|a| a == 5} + # => true + # {}.all?{|a| a != 5} + # => true class CanDefinition - def matches_conditions_hash?(subject, conditions = @conditions) - if subject.class.include?(Mongoid::Document) # Mongoid Criteria are simpler to check than normal conditions hashes - if conditions.empty? # When no conditions are given, true should be returned. - # The default CanCan behavior relies on the fact that conditions.all? will return true when conditions is empty - # The way ruby handles all? for empty hashes can be unexpected: - # {}.all?{|a| a == 5} - # => true - # {}.all?{|a| a != 5} - # => true + def matches_conditions_hash_with_mongoid_subject?(subject, conditions = @conditions) + if subject.class.include?(Mongoid::Document) && conditions.any?{|k,v| !k.kind_of?(Symbol)} + if conditions.empty? true else subject.class.where(conditions).include?(subject) # just use Mongoid's where function end else - conditions.all? do |name, value| - attribute = subject.send(name) - if value.kind_of?(Hash) - if attribute.kind_of? Array - attribute.any? { |element| matches_conditions_hash? element, value } - else - matches_conditions_hash? attribute, value - end - elsif value.kind_of?(Array) || value.kind_of?(Range) - value.include? attribute - else - attribute == value - end - end + matches_conditions_hash_without_mongoid_subject? subject, conditions end end + alias_method :matches_conditions_hash_without_mongoid_subject?, :matches_conditions_hash? + alias_method :matches_conditions_hash?, :matches_conditions_hash_with_mongoid_subject? end @@ -114,4 +105,4 @@ if defined? Mongoid end end end -end \ No newline at end of file +end diff --git a/spec/cancan/mongoid_additions_spec.rb b/spec/cancan/mongoid_additions_spec.rb index d078c99..ba9da0b 100644 --- a/spec/cancan/mongoid_additions_spec.rb +++ b/spec/cancan/mongoid_additions_spec.rb @@ -49,6 +49,12 @@ describe CanCan::MongoidAdditions do collection.name !~ /system/ end.each(&:drop) end + + it "should compare properties on mongoid documents with the " do + model = @model_class.new + @ability.can :read, @model_class, :id => model.id + @ability.should be_able_to :read, model + end it "should return [] when no ability is defined so no records are found" do @model_class.create :title => 'Sir' @@ -88,6 +94,22 @@ describe CanCan::MongoidAdditions do @ability.can?(:read, obj2).should == false end + describe "activates only when there are Criteria in the hash" do + it "Calls where on the model class when there are criteria" do + obj = @model_class.create :title => 'Bird' + @conditions = {:title.nin => ["Fork", "Spoon"]} + @model_class.should_receive(:where).with(@conditions) {[obj]} + @ability.can :read, @model_class, @conditions + @ability.can?(:read, obj) + end + it "Calls the base version if there are no mongoid criteria" do + obj = @model_class.new :title => 'Bird' + @conditions = {:id => obj.id} + @ability.can :read, @model_class, @conditions + @ability.should be_able_to(:read, obj) + end + end + it "should handle :field.nin" do obj = @model_class.create :title => 'Sir' @ability.can :read, @model_class, :title.nin => ["Lord", "Madam"] @@ -141,4 +163,4 @@ describe CanCan::MongoidAdditions do @model_class.accessible_by(@ability) }.should raise_error(CanCan::Error) end -end \ No newline at end of file +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index da6b77b..0d0be82 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,9 +5,11 @@ require 'supermodel' # shouldn't Bundler do this already? require 'active_support/all' require 'matchers' require 'cancan/matchers' +require 'mongoid' +require 'active_support' RSpec.configure do |config| - config.mock_with :rr + config.mock_with :rspec config.before(:each) do Project.delete_all Category.delete_all