From 7c99c593dd55127db16b0920167814a296c38e0d Mon Sep 17 00:00:00 2001 From: Alessandro Rodi Date: Wed, 6 Jul 2022 17:18:04 +0200 Subject: [PATCH] Allow to disable rules compressor (#798) --- CHANGELOG.md | 1 + docs/rules_compression.md | 8 +++++- lib/cancan/config.rb | 23 ++++++++++++++++ .../model_adapters/active_record_adapter.rb | 10 ++++--- .../active_record_adapter_spec.rb | 26 ++++++++++++++----- spec/spec_helper.rb | 1 + 6 files changed, 59 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6178c6ec1..5dece0e08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * [#653](https://github.com/CanCanCommunity/cancancan/pull/653): Add support for using an nil relation as a condition. ([@ghiculescu][]) * [#702](https://github.com/CanCanCommunity/cancancan/pull/702): Support scopes of STI classes as ability conditions. ([@honigc][]) +* [#798](https://github.com/CanCanCommunity/cancancan/pull/798): Allow disabling of rules compressor via `CanCan.rules_compressor_enabled = false`. ([@coorasse][]) ## 3.4.0 diff --git a/docs/rules_compression.md b/docs/rules_compression.md index 077decec6..a535dbf37 100644 --- a/docs/rules_compression.md +++ b/docs/rules_compression.md @@ -1,6 +1,12 @@ # Rules compressions -Your rules are optimized automatically at runtime. There are a set of "rules" to optimize your rules definition and they are implemented in the `RulesCompressor` class. Here you can see how this works: +Database are great on optimizing queries, but sometimes cancancan builds `joins` that might lead to slow performance. +This is why your rules are optimized automatically at runtime. +There are a set of "rules" to optimize your rules definition and they are implemented in the `RulesCompressor` class. +You can always disable the rules compressor by setting `CanCan.rules_compressor_enabled = false` in your initializer. +You can also enable/disable it on a specific check by using: `with_rules_compressor_enabled(false) { ... }` + +Here you can see how this works: A rule without conditions is defined as `catch_all`. diff --git a/lib/cancan/config.rb b/lib/cancan/config.rb index 75e68bad7..0fd8893f8 100644 --- a/lib/cancan/config.rb +++ b/lib/cancan/config.rb @@ -11,6 +11,29 @@ def self.valid_accessible_by_strategies strategies end + # You can disable the rules compressor if it's causing unexpected issues. + def self.rules_compressor_enabled + return @rules_compressor_enabled if defined?(@rules_compressor_enabled) + + @rules_compressor_enabled = true + end + + def self.rules_compressor_enabled=(value) + @rules_compressor_enabled = value + end + + def self.with_rules_compressor_enabled(value) + return yield if value == rules_compressor_enabled + + begin + rules_compressor_enabled_was = rules_compressor_enabled + @rules_compressor_enabled = value + yield + ensure + @rules_compressor_enabled = rules_compressor_enabled_was + end + end + # Determines how CanCan should build queries when calling accessible_by, # if the query will contain a join. The default strategy is `:subquery`. # diff --git a/lib/cancan/model_adapters/active_record_adapter.rb b/lib/cancan/model_adapters/active_record_adapter.rb index 2bf2941d3..9eeefdcb7 100644 --- a/lib/cancan/model_adapters/active_record_adapter.rb +++ b/lib/cancan/model_adapters/active_record_adapter.rb @@ -15,7 +15,11 @@ def self.version_lower?(version) def initialize(model_class, rules) super - @compressed_rules = RulesCompressor.new(@rules.reverse).rules_collapsed.reverse + @compressed_rules = if CanCan.rules_compressor_enabled + RulesCompressor.new(@rules.reverse).rules_collapsed.reverse + else + @rules + end StiNormalizer.normalize(@compressed_rules) ConditionsNormalizer.normalize(model_class, @compressed_rules) end @@ -39,8 +43,8 @@ def nested_subject_matches_conditions?(parent, child, all_conditions) def parent_child_conditions(parent, child, all_conditions) child_class = child.is_a?(Class) ? child : child.class foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association| - association.klass == parent.class - end&.foreign_key&.to_sym + association.klass == parent.class + end&.foreign_key&.to_sym foreign_key.nil? ? nil : all_conditions[foreign_key] end end diff --git a/spec/cancan/model_adapters/active_record_adapter_spec.rb b/spec/cancan/model_adapters/active_record_adapter_spec.rb index b1a76ebdd..efb03c629 100644 --- a/spec/cancan/model_adapters/active_record_adapter_spec.rb +++ b/spec/cancan/model_adapters/active_record_adapter_spec.rb @@ -393,12 +393,26 @@ class User < ActiveRecord::Base expect(@ability.model_adapter(Article, :read).joins).to be_nil end - it 'has nil joins if rules got compressed' do - @ability.can :read, Comment, article: { category: { visible: true } } - @ability.can :read, Comment - expect(@ability.model_adapter(Comment, :read)) - .to generate_sql("SELECT \"#{@comment_table}\".* FROM \"#{@comment_table}\"") - expect(@ability.model_adapter(Comment, :read).joins).to be_nil + context 'if rules got compressed' do + it 'has nil joins' do + @ability.can :read, Comment, article: { category: { visible: true } } + @ability.can :read, Comment + expect(@ability.model_adapter(Comment, :read)) + .to generate_sql("SELECT \"#{@comment_table}\".* FROM \"#{@comment_table}\"") + expect(@ability.model_adapter(Comment, :read).joins).to be_nil + end + end + + context 'if rules did not get compressed' do + before :each do + CanCan.rules_compressor_enabled = false + end + + it 'has joins' do + @ability.can :read, Comment, article: { category: { visible: true } } + @ability.can :read, Comment + expect(@ability.model_adapter(Comment, :read).joins).to be_present + end end it 'has nil joins if no nested hashes specified in conditions' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7f2a4ab8c..72b4ac23e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -30,6 +30,7 @@ config.after :each do CanCan.accessible_by_strategy = CanCan.default_accessible_by_strategy + CanCan.rules_compressor_enabled = true end end