Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessible by strategy: Exists subquery #691

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/cancan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
require 'cancan/model_adapters/active_record_4_adapter'
require 'cancan/model_adapters/active_record_5_adapter'
require 'cancan/model_adapters/strategies/base'
require 'cancan/model_adapters/strategies/joined_alias_each_rule_as_exists_subquery'
require 'cancan/model_adapters/strategies/joined_alias_exists_subquery'
require 'cancan/model_adapters/strategies/left_join'
require 'cancan/model_adapters/strategies/subquery'
end
6 changes: 5 additions & 1 deletion lib/cancan/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
module CanCan
def self.valid_accessible_by_strategies
strategies = [:left_join]
strategies << :subquery unless does_not_support_subquery_strategy?

unless does_not_support_subquery_strategy?
strategies.push(:joined_alias_exists_subquery, :joined_alias_each_rule_as_exists_subquery, :subquery)
end

strategies
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
module CanCan
module ModelAdapters
class Strategies
class JoinedAliasEachRuleAsExistsSubquery < Base
def execute!
model_class
.joins(
"JOIN #{quoted_table_name} AS #{quoted_aliased_table_name} ON " \
"#{quoted_aliased_table_name}.#{quoted_primary_key} = #{quoted_table_name}.#{quoted_primary_key}"
)
.where(double_exists_sql)
end

def double_exists_sql
double_exists_sql = ''

compressed_rules.each_with_index do |rule, index|
double_exists_sql << ' OR ' if index > 0
double_exists_sql << "EXISTS (#{sub_query_for_rule(rule).to_sql})"
end

double_exists_sql
end

def sub_query_for_rule(rule)
conditions_extractor = ConditionsExtractor.new(model_class)
rule_where_conditions = extract_multiple_conditions(conditions_extractor, [rule])
joins_hash, left_joins_hash = extract_joins_from_rule(rule)
sub_query_for_rules_and_join_hashes(rule_where_conditions, joins_hash, left_joins_hash)
end

def sub_query_for_rules_and_join_hashes(rule_where_conditions, joins_hash, left_joins_hash)
model_class
.select('1')
.joins(joins_hash)
.left_joins(left_joins_hash)
.where(
"#{quoted_table_name}.#{quoted_primary_key} = " \
"#{quoted_aliased_table_name}.#{quoted_primary_key}"
)
.where(rule_where_conditions)
.limit(1)
end

def extract_joins_from_rule(rule)
joins = {}
left_joins = {}

extra_joins_recursive([], rule.conditions, joins, left_joins)
[joins, left_joins]
end

def extra_joins_recursive(current_path, conditions, joins, left_joins)
conditions.each do |key, value|
if value.is_a?(Hash)
current_path << key
extra_joins_recursive(current_path, value, joins, left_joins)
current_path.pop
else
extra_joins_recursive_merge_joins(current_path, value, joins, left_joins)
end
end
end

def extra_joins_recursive_merge_joins(current_path, value, joins, left_joins)
hash_joins = current_path_to_hash(current_path)

if value.nil?
left_joins.deep_merge!(hash_joins)
else
joins.deep_merge!(hash_joins)
end
end

# Converts an array like [:child, :grand_child] into a hash like {child: {grand_child: {}}
def current_path_to_hash(current_path)
hash_joins = {}
current_hash_joins = hash_joins

current_path.each do |path_part|
new_hash = {}
current_hash_joins[path_part] = new_hash
current_hash_joins = new_hash
end

hash_joins
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module CanCan
module ModelAdapters
class Strategies
class JoinedAliasExistsSubquery < Base
def execute!
model_class
.joins(
"JOIN #{quoted_table_name} AS #{quoted_aliased_table_name} ON " \
"#{quoted_aliased_table_name}.#{quoted_primary_key} = #{quoted_table_name}.#{quoted_primary_key}"
)
.where("EXISTS (#{joined_alias_exists_subquery_inner_query.to_sql})")
end

def joined_alias_exists_subquery_inner_query
model_class
.unscoped
.select('1')
.left_joins(joins)
.where(*where_conditions)
.where(
"#{quoted_table_name}.#{quoted_primary_key} = " \
"#{quoted_aliased_table_name}.#{quoted_primary_key}"
)
.limit(1)
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class Editor < ActiveRecord::Base
if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
describe 'selecting custom columns' do
it 'extracts custom columns correctly' do
posts = Post.accessible_by(ability).where(published: true).select('title as mytitle')
posts = Post.accessible_by(ability).where(published: true).select('posts.title as mytitle')
expect(posts[0].mytitle).to eq 'post1'
end
end
Expand Down
56 changes: 56 additions & 0 deletions spec/cancan/model_adapters/has_and_belongs_to_many_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,62 @@ class House < ActiveRecord::Base
end

unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
describe 'fetching of records - joined_alias_subquery strategy' do
before do
CanCan.accessible_by_strategy = :joined_alias_exists_subquery
end

it 'it retreives the records correctly' do
houses = House.accessible_by(ability)
expect(houses).to match_array [@house2, @house1]
end

if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
it 'generates the correct query' do
expect(ability.model_adapter(House, :read))
.to generate_sql("SELECT \"houses\".*
FROM \"houses\"
JOIN \"houses\" AS \"houses_alias\" ON \"houses_alias\".\"id\" = \"houses\".\"id\"
WHERE (EXISTS (SELECT 1
FROM \"houses\"
LEFT OUTER JOIN \"houses_people\" ON \"houses_people\".\"house_id\" = \"houses\".\"id\"
LEFT OUTER JOIN \"people\" ON \"people\".\"id\" = \"houses_people\".\"person_id\"
WHERE
\"people\".\"id\" = #{@person1.id} AND
(\"houses\".\"id\" = \"houses_alias\".\"id\") LIMIT 1))
")
end
end
end

describe 'fetching of records - joined_alias_each_rule_as_exists_subquery strategy' do
before do
CanCan.accessible_by_strategy = :joined_alias_each_rule_as_exists_subquery
end

it 'it retreives the records correctly' do
houses = House.accessible_by(ability)
expect(houses).to match_array [@house2, @house1]
end

if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
it 'generates the correct query' do
expect(ability.model_adapter(House, :read))
.to generate_sql("SELECT \"houses\".*
FROM \"houses\"
JOIN \"houses\" AS \"houses_alias\" ON \"houses_alias\".\"id\" = \"houses\".\"id\"
WHERE (EXISTS (SELECT 1
FROM \"houses\"
INNER JOIN \"houses_people\" ON \"houses_people\".\"house_id\" = \"houses\".\"id\"
INNER JOIN \"people\" ON \"people\".\"id\" = \"houses_people\".\"person_id\"
WHERE (\"houses\".\"id\" = \"houses_alias\".\"id\") AND
(\"people\".\"id\" = #{@person1.id})
LIMIT 1))
")
end
end
end

describe 'fetching of records - subquery strategy' do
before do
CanCan.accessible_by_strategy = :subquery
Expand Down