From 15ca8ade3b6accf92a8c4b18926e0c53a19f0040 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Wed, 5 Jan 2011 13:22:06 -0800 Subject: [PATCH] improving DataMapper adapter and specs --- Gemfile | 2 + LICENSE | 2 +- .../model_adapters/data_mapper_adapter.rb | 10 +- lib/cancan/model_adapters/mongoid_adapter.rb | 2 +- .../data_mapper_adapter_spec.rb | 129 ++++++++++++------ 5 files changed, 102 insertions(+), 43 deletions(-) diff --git a/Gemfile b/Gemfile index 1c0d3d8..82993b1 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,8 @@ when nil, "active_record" gem "with_model" when "data_mapper" gem "dm-core", "~> 1.0.2" + gem "dm-sqlite-adapter", "~> 1.0.2" + gem "dm-migrations", "~> 1.0.2" when "mongoid" gem "bson_ext", "~> 1.1" gem "mongoid", "~> 2.0.0.beta.19" diff --git a/LICENSE b/LICENSE index bc77690..485ab61 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2010 Ryan Bates +Copyright (c) 2011 Ryan Bates Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/lib/cancan/model_adapters/data_mapper_adapter.rb b/lib/cancan/model_adapters/data_mapper_adapter.rb index 417bedc..9d456d2 100644 --- a/lib/cancan/model_adapters/data_mapper_adapter.rb +++ b/lib/cancan/model_adapters/data_mapper_adapter.rb @@ -5,8 +5,16 @@ module CanCan model_class <= DataMapper::Resource end + def self.override_conditions_hash_matching?(subject, conditions) + conditions.any? { |k,v| !k.kind_of?(Symbol) } + end + + def self.matches_conditions_hash?(subject, conditions) + subject.class.all(:conditions => conditions).include?(subject) # TODO don't use a database query here for performance and other instances + end + def database_records - scope = @model_class.all(:conditions => ['true=false']) + scope = @model_class.all(:conditions => ["0=1"]) conditions.each do |condition| scope += @model_class.all(:conditions => condition) end diff --git a/lib/cancan/model_adapters/mongoid_adapter.rb b/lib/cancan/model_adapters/mongoid_adapter.rb index ced2a41..19b8e45 100644 --- a/lib/cancan/model_adapters/mongoid_adapter.rb +++ b/lib/cancan/model_adapters/mongoid_adapter.rb @@ -10,7 +10,7 @@ module CanCan end def self.matches_conditions_hash?(subject, conditions) - subject.class.where(conditions).include?(subject) # just use Mongoid's where function + subject.class.where(conditions).include?(subject) # TODO don't use a database query here for performance and other instances end def database_records diff --git a/spec/cancan/model_adapters/data_mapper_adapter_spec.rb b/spec/cancan/model_adapters/data_mapper_adapter_spec.rb index dbafbdf..924b65f 100644 --- a/spec/cancan/model_adapters/data_mapper_adapter_spec.rb +++ b/spec/cancan/model_adapters/data_mapper_adapter_spec.rb @@ -1,66 +1,115 @@ if ENV["MODEL_ADAPTER"] == "data_mapper" require "spec_helper" + DataMapper.setup(:default, 'sqlite::memory:') + + class Article + include DataMapper::Resource + property :id, Serial + property :published, Boolean, :default => false + property :secret, Boolean, :default => false + property :priority, Integer + has n, :comments + end + + class Comment + include DataMapper::Resource + property :id, Serial + property :spam, Boolean, :default => false + belongs_to :article + end + + DataMapper.finalize + DataMapper.auto_migrate! + describe CanCan::ModelAdapters::DataMapperAdapter do before(:each) do - @model_class = Class.new - @model_class.class_eval do - include DataMapper::Resource - end - stub(@model_class).all(:conditions => ['true=false']) { 'no-match:' } - + Article.destroy + Comment.destroy @ability = Object.new @ability.extend(CanCan::Ability) end it "should be for only data mapper classes" do CanCan::ModelAdapters::DataMapperAdapter.should_not be_for_class(Object) - CanCan::ModelAdapters::DataMapperAdapter.should be_for_class(@model_class) - CanCan::ModelAdapters::AbstractAdapter.adapter_class(@model_class).should == CanCan::ModelAdapters::DataMapperAdapter + CanCan::ModelAdapters::DataMapperAdapter.should be_for_class(Article) + CanCan::ModelAdapters::AbstractAdapter.adapter_class(Article).should == CanCan::ModelAdapters::DataMapperAdapter end - it "should return no records when no ability is defined so no records are found" do - @model_class.accessible_by(@ability, :read).should == 'no-match:' + it "should not fetch any records when no abilities are defined" do + Article.create + Article.accessible_by(@ability).should be_empty end - it "should call all with matching ability conditions" do - @ability.can :read, @model_class, :foo => {:bar => 1} - stub(@model_class).all(:conditions => {:foo => {:bar => 1}}) { 'found-records:' } - @model_class.accessible_by(@ability, :read).should == 'no-match:found-records:' + it "should fetch all articles when one can read all" do + @ability.can :read, Article + article = Article.create + Article.accessible_by(@ability).should == [article] end - it "should merge association joins and sanitize conditions" do - @ability.can :read, @model_class, :foo => {:bar => 1} - @ability.can :read, @model_class, :too => {:car => 1, :far => {:bar => 1}} - - stub(@model_class).all(:conditions => {:foo => {:bar => 1}}) { 'foo:' } - stub(@model_class).all(:conditions => {:too => {:car => 1, :far => {:bar => 1}}}) { 'too:' } - - @model_class.accessible_by(@ability).should == 'no-match:too:foo:' + it "should fetch only the articles that are published" do + @ability.can :read, Article, :published => true + article1 = Article.create(:published => true) + article2 = Article.create(:published => false) + Article.accessible_by(@ability).should == [article1] end - it "should allow to define sql conditions by not hash" do - @ability.can :read, @model_class, :foo => 1 - @ability.can :read, @model_class, ['bar = ?', 1] - - stub(@model_class).all(:conditions => {:foo => 1}) { 'foo:' } - stub(@model_class).all(:conditions => ['bar = ?', 1]) { 'bar:' } - - @model_class.accessible_by(@ability).should == 'no-match:bar:foo:' + it "should fetch any articles which are published or secret" do + @ability.can :read, Article, :published => true + @ability.can :read, Article, :secret => true + article1 = Article.create(:published => true, :secret => false) + article2 = Article.create(:published => true, :secret => true) + article3 = Article.create(:published => false, :secret => true) + article4 = Article.create(:published => false, :secret => false) + Article.accessible_by(@ability).should == [article1, article2, article3] end - it "should not allow to fetch records when ability with just block present" do - @ability.can :read, @model_class do false end - lambda { - @model_class.accessible_by(@ability) - }.should raise_error(CanCan::Error) + it "should fetch only the articles that are published and not secret" do + pending "the `cannot` may require some custom SQL, maybe abstract out from Active Record adapter" + @ability.can :read, Article, :published => true + @ability.cannot :read, Article, :secret => true + article1 = Article.create(:published => true, :secret => false) + article2 = Article.create(:published => true, :secret => true) + article3 = Article.create(:published => false, :secret => true) + article4 = Article.create(:published => false, :secret => false) + Article.accessible_by(@ability).should == [article1] end - it "should not allow to check ability on object when nonhash sql ability definition without block present" do - @ability.can :read, @model_class, ['bar = ?', 1] - lambda { - @ability.can? :read, @model_class.new - }.should raise_error(CanCan::Error) + it "should only read comments for articles which are published" do + @ability.can :read, Comment, :article => { :published => true } + comment1 = Comment.create(:article => Article.create!(:published => true)) + comment2 = Comment.create(:article => Article.create!(:published => false)) + Comment.accessible_by(@ability).should == [comment1] end + + it "should allow conditions in SQL and merge with hash conditions" do + @ability.can :read, Article, :published => true + @ability.can :read, Article, ["secret=?", true] + article1 = Article.create(:published => true, :secret => false) + article4 = Article.create(:published => false, :secret => false) + Article.accessible_by(@ability).should == [article1] + end + + it "should match gt comparison" do + @ability.can :read, Article, :priority.gt => 3 + article1 = Article.create(:priority => 4) + article2 = Article.create(:priority => 3) + Article.accessible_by(@ability).should == [article1] + @ability.should be_able_to(:read, article1) + @ability.should_not be_able_to(:read, article2) + end + + it "should match gte comparison" do + @ability.can :read, Article, :priority.gte => 3 + article1 = Article.create(:priority => 4) + article2 = Article.create(:priority => 3) + article3 = Article.create(:priority => 2) + Article.accessible_by(@ability).should == [article1, article2] + @ability.should be_able_to(:read, article1) + @ability.should be_able_to(:read, article2) + @ability.should_not be_able_to(:read, article3) + end + + # TODO: add more comparison specs end end