Skip to content

Commit

Permalink
Merge pull request #675 from ghiculescu/strategy-per-query
Browse files Browse the repository at this point in the history
Support modifying the `accessible_by` querying strategy on a per-query basis
  • Loading branch information
coorasse authored Mar 14, 2021
2 parents 2782e29 + c220e87 commit 3e7bc1a
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased

* [#675](https://github.com/CanCanCommunity/cancancan/pull/675): Support modifying the `accessible_by` querying strategy on a per-query basis. ([@ghiculescu][])

## 3.2.1

* [#674](https://github.com/CanCanCommunity/cancancan/pull/674): Fix accidental dependency on ActiveRecord in 3.2.0. ([@ghiculescu][])
Expand Down
28 changes: 24 additions & 4 deletions lib/cancan/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ def self.valid_accessible_by_strategies
# `distinct` is not reliable in some cases. See
# https://github.com/CanCanCommunity/cancancan/pull/605
def self.accessible_by_strategy
@accessible_by_strategy || default_accessible_by_strategy
return @accessible_by_strategy if @accessible_by_strategy

@accessible_by_strategy = default_accessible_by_strategy
end

def self.default_accessible_by_strategy
Expand All @@ -36,9 +38,7 @@ def self.default_accessible_by_strategy
end

def self.accessible_by_strategy=(value)
unless valid_accessible_by_strategies.include?(value)
raise ArgumentError, "accessible_by_strategy must be one of #{valid_accessible_by_strategies.join(', ')}"
end
validate_accessible_by_strategy!(value)

if value == :subquery && does_not_support_subquery_strategy?
raise ArgumentError, 'accessible_by_strategy = :subquery requires ActiveRecord 5 or newer'
Expand All @@ -47,6 +47,26 @@ def self.accessible_by_strategy=(value)
@accessible_by_strategy = value
end

def self.with_accessible_by_strategy(value)
return yield if value == accessible_by_strategy

validate_accessible_by_strategy!(value)

begin
strategy_was = accessible_by_strategy
@accessible_by_strategy = value
yield
ensure
@accessible_by_strategy = strategy_was
end
end

def self.validate_accessible_by_strategy!(value)
return if valid_accessible_by_strategies.include?(value)

raise ArgumentError, "accessible_by_strategy must be one of #{valid_accessible_by_strategies.join(', ')}"
end

def self.does_not_support_subquery_strategy?
!defined?(CanCan::ModelAdapters::ActiveRecordAdapter) ||
CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
Expand Down
6 changes: 4 additions & 2 deletions lib/cancan/model_additions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ module ClassMethods
# @articles = Article.accessible_by(current_ability, :update)
#
# Here only the articles which the user can update are returned.
def accessible_by(ability, action = :index)
ability.model_adapter(self, action).database_records
def accessible_by(ability, action = :index, strategy: CanCan.accessible_by_strategy)
CanCan.with_accessible_by_strategy(strategy) do
ability.model_adapter(self, action).database_records
end
end
end

Expand Down
95 changes: 95 additions & 0 deletions spec/cancan/model_adapters/active_record_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,101 @@ class Transaction < ActiveRecord::Base
end
end

if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
context 'switching strategies' do
before do
CanCan.accessible_by_strategy = :left_join # default - should be ignored in these tests
end

it 'allows you to switch strategies with a keyword argument' do
u = User.create!(name: 'pippo')
Article.create!(mentioned_users: [u])

ability = Ability.new(u)
ability.can :read, Article, mentions: { user: { name: u.name } }

subquery_sql = Article.accessible_by(ability, strategy: :subquery).to_sql
left_join_sql = Article.accessible_by(ability, strategy: :left_join).to_sql

expect(subquery_sql.strip.squeeze(' ')).to eq(%(
SELECT "articles".*
FROM "articles"
WHERE "articles"."id" IN
(SELECT "articles"."id"
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo')
).gsub(/\s+/, ' ').strip)

expect(left_join_sql.strip.squeeze(' ')).to eq(%(
SELECT DISTINCT "articles".*
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip)
end

it 'allows you to switch strategies with a block' do
u = User.create!(name: 'pippo')
Article.create!(mentioned_users: [u])

ability = Ability.new(u)
ability.can :read, Article, mentions: { user: { name: u.name } }

subquery_sql = CanCan.with_accessible_by_strategy(:subquery) { Article.accessible_by(ability).to_sql }
left_join_sql = CanCan.with_accessible_by_strategy(:left_join) { Article.accessible_by(ability).to_sql }

expect(subquery_sql.strip.squeeze(' ')).to eq(%(
SELECT "articles".*
FROM "articles"
WHERE "articles"."id" IN
(SELECT "articles"."id"
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo')
).gsub(/\s+/, ' ').strip)

expect(left_join_sql.strip.squeeze(' ')).to eq(%(
SELECT DISTINCT "articles".*
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip)
end

it 'allows you to switch strategies with a block, and to_sql called outside the block' do
u = User.create!(name: 'pippo')
Article.create!(mentioned_users: [u])

ability = Ability.new(u)
ability.can :read, Article, mentions: { user: { name: u.name } }

subquery_sql = CanCan.with_accessible_by_strategy(:subquery) { Article.accessible_by(ability) }.to_sql
left_join_sql = CanCan.with_accessible_by_strategy(:left_join) { Article.accessible_by(ability) }.to_sql

expect(subquery_sql.strip.squeeze(' ')).to eq(%(
SELECT "articles".*
FROM "articles"
WHERE "articles"."id" IN
(SELECT "articles"."id"
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo')
).gsub(/\s+/, ' ').strip)

expect(left_join_sql.strip.squeeze(' ')).to eq(%(
SELECT DISTINCT "articles".*
FROM "articles"
LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id"
LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id"
WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip)
end
end
end

CanCan.valid_accessible_by_strategies.each do |strategy|
context "when a model has renamed primary_key with #{strategy} strategy" do
before :each do
Expand Down

0 comments on commit 3e7bc1a

Please sign in to comment.