Merge branch 'develop'

.rubocop.yml

@@ -6,6 +6,9 @@ Style/Documentation:
 Style/NonNilCheck:
   IncludeSemanticChanges: true
 
+Style/RedundantInitialize:
+  Enabled: false
+
 Style/EmptyMethod:
   Enabled: false
 

CHANGELOG.md

@@ -1,4 +1,9 @@
-## Unreleased
+## 3.5.0
+
+* [#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][])
+* [#814](https://github.com/CanCanCommunity/cancancan/pull/814): Fix issue with polymorphic associations. ([@WriterZephos][])
 
 ## 3.4.0
 
@@ -700,3 +705,5 @@ Please read the [guide on migrating from CanCanCan 2.x to 3.0](https://github.co
 [@ghiculescu]: https://github.com/ghiculescu
 [@mtoneil]: https://github.com/mtoneil
 [@Juleffel]: https://github.com/Juleffel
+[@honigc]: https://github.com/honigc
+[@WriterZephos]: https://github.com/WriterZephos

README.md

@@ -7,7 +7,7 @@
 [![Code Climate Badge](https://codeclimate.com/github/CanCanCommunity/cancancan.svg)](https://codeclimate.com/github/CanCanCommunity/cancancan)
 
 [Developer guide](./docs/README.md) |
-[RDocs](http://rdoc.info/projects/CanCanCommunity/cancancan) |
+[RDocs](https://www.rubydoc.info/github/CanCanCommunity/cancancan) |
 [Screencast 1](http://railscasts.com/episodes/192-authorization-with-cancan) |
 [Screencast 2](https://www.youtube.com/watch?v=cTYu-OjUgDw)
 
@@ -179,6 +179,8 @@ When first developing, you need to run `bundle install` and then `bundle exec ap
 
 You can then run all appraisal files (like CI does), with `appraisal rake` or just run a specific set `DB='sqlite' bundle exec appraisal activerecord_5.2.2 rake`.
 
+If you'd like to run a specific set of tests within a specific file or folder you can use `DB='sqlite' SPEC=path/to/file/or/folder bundle exec appraisal activerecord_5.2.2 rake`.
+
 If you use RubyMine, you can run RSpec tests by configuring the RSpec configuration template like this:
 ![rubymine_rspec.png](rubymine_rspec.png)
 

cancancan.gemspec

@@ -25,5 +25,5 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'bundler', '~> 2.0'
   s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.1'
   s.add_development_dependency 'rspec', '~> 3.2', '>= 3.2.0'
-  s.add_development_dependency 'rubocop', '~> 1.26'
+  s.add_development_dependency 'rubocop', '~> 1.31.1'
 end

docs/friendly_id.md

@@ -4,7 +4,7 @@ If you are using [FriendlyId](https://github.com/norman/friendly_id) you will pr
 
 You do not have to write `find_by :slug` or something like that, that is always error prone.
 
-You just need to create a `config/initizializers/cancancan.rb` file with:
+You just need to create a `config/initializers/cancancan.rb` file with:
 
 ```ruby
 if defined?(CanCanCan)

docs/hash_of_conditions.md

@@ -46,6 +46,13 @@ An array or range can be passed to match multiple values. Here the user can only
 can :read, Project, priority: 1..3
 ```
 
+If you want to a negative match, you can pass in `nil`.
+
+```ruby
+# Can read projects that don't have any members.
+can :read, Project, members: { id: nil }
+```
+
 Almost anything that you can pass to a hash of conditions in ActiveRecord will work here as well.
 
 ## Traverse associations

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`.
 

lib/cancan/ability/rules.rb

@@ -61,8 +61,8 @@ def relevant_rules_for_match(action, subject)
           next unless rule.only_raw_sql?
 
           raise Error,
-                "The can? and cannot? call cannot be used with a raw sql 'can' definition."\
-                " The checking code cannot be determined for #{action.inspect} #{subject.inspect}"
+                "The can? and cannot? call cannot be used with a raw sql 'can' definition. " \
+                "The checking code cannot be determined for #{action.inspect} #{subject.inspect}"
         end
       end
 
@@ -72,7 +72,7 @@ def relevant_rules_for_query(action, subject)
           rule.base_behavior == false && rule.attributes.present?
         end
         if rules.any?(&:only_block?)
-          raise Error, "The accessible_by call cannot be used with a block 'can' definition."\
+          raise Error, "The accessible_by call cannot be used with a block 'can' definition." \
             "The SQL cannot be determined for #{action.inspect} #{subject.inspect}"
         end
         rules

lib/cancan/conditions_matcher.rb

@@ -18,10 +18,14 @@ def subject_class?(subject)
       [Class, Module].include? klass
     end
 
-    def matches_block_conditions(subject, *extra_args)
+    def matches_block_conditions(subject, attribute, *extra_args)
       return @base_behavior if subject_class?(subject)
 
-      @block.call(subject, *extra_args.compact)
+      if attribute
+        @block.call(subject, attribute, *extra_args)
+      else
+        @block.call(subject, *extra_args)
+      end
     end
 
     def matches_non_block_conditions(subject)
@@ -35,11 +39,13 @@ def matches_non_block_conditions(subject)
     def nested_subject_matches_conditions?(subject_hash)
       parent, child = subject_hash.first
 
-      matches_base_parent_conditions = matches_conditions_hash?(parent,
-                                                                @conditions[parent.class.name.downcase.to_sym] || {})
-
       adapter = model_adapter(parent)
 
+      parent_condition_name = adapter.parent_condition_name(parent, child)
+
+      matches_base_parent_conditions = matches_conditions_hash?(parent,
+                                                                @conditions[parent_condition_name] || {})
+
       matches_base_parent_conditions &&
         (!adapter.override_nested_subject_conditions_matching?(parent, child, @conditions) ||
           adapter.nested_subject_matches_conditions?(parent, child, @conditions))
@@ -63,16 +69,15 @@ def matches_conditions_hash?(subject, conditions = @conditions)
 
     def matches_all_conditions?(adapter, subject, conditions)
       if conditions.is_a?(Hash)
-        matches_hash_conditions(adapter, subject, conditions)
+        matches_hash_conditions?(adapter, subject, conditions)
       elsif conditions.respond_to?(:include?)
         conditions.include?(subject)
       else
-        puts "does #{subject} match #{conditions}?"
         subject == conditions
       end
     end
 
-    def matches_hash_conditions(adapter, subject, conditions)
+    def matches_hash_conditions?(adapter, subject, conditions)
       conditions.all? do |name, value|
         if adapter.override_condition_matching?(subject, name, value)
           adapter.matches_condition?(subject, name, value)
@@ -97,12 +102,29 @@ def condition_match?(attribute, value)
 
     def hash_condition_match?(attribute, value)
       if attribute.is_a?(Array) || (defined?(ActiveRecord) && attribute.is_a?(ActiveRecord::Relation))
-        attribute.to_a.any? { |element| matches_conditions_hash?(element, value) }
+        array_like_matches_condition_hash?(attribute, value)
       else
         attribute && matches_conditions_hash?(attribute, value)
       end
     end
 
+    def array_like_matches_condition_hash?(attribute, value)
+      if attribute.any?
+        attribute.any? { |element| matches_conditions_hash?(element, value) }
+      else
+        # you can use `nil`s in your ability definition to tell cancancan to find
+        # objects that *don't* have any children in a has_many relationship.
+        #
+        # for example, given ability:
+        # => can :read, Article, comments: { id: nil }
+        # cancancan will return articles where `article.comments == []`
+        #
+        # this is implemented here. `attribute` is `article.comments`, and it's an empty array.
+        # the expression below returns true if this was expected.
+        !value.values.empty? && value.values.all?(&:nil?)
+      end
+    end
+
     def call_block_with_all(action, subject, *extra_args)
       if subject.class == Class
         @block.call(action, subject, nil, *extra_args)

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`.
   #

lib/cancan/controller_additions.rb

@@ -264,7 +264,7 @@ def check_authorization(options = {})
           next if options[:unless] && controller.send(options[:unless])
 
           raise AuthorizationNotPerformed,
-                'This action failed the check_authorization because it does not authorize_resource. '\
+                'This action failed the check_authorization because it does not authorize_resource. ' \
                 'Add skip_authorization_check to bypass this check.'
         end
 

lib/cancan/model_adapters/abstract_adapter.rb

@@ -35,6 +35,11 @@ def self.matches_conditions_hash?(_subject, _conditions)
         raise NotImplemented, 'This model adapter does not support matching on a conditions hash.'
       end
 
+      # Override if parent condition could be under a different key in conditions
+      def self.parent_condition_name(parent, _child)
+        parent.class.name.downcase.to_sym
+      end
+
       # Used above override_conditions_hash_matching to determine if this model adapter will override the
       # matching behavior for nested subject.
       # If this returns true then nested_subject_matches_conditions? will be called.

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
@@ -38,11 +42,53 @@ 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
+          parent_class = parent.is_a?(Class) ? parent : parent.class
+
           foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association|
-                          association.klass == parent.class
-                        end&.foreign_key&.to_sym
+            # Do not match on polymorphic associations or it will throw an error (klass cannot be determined)
+            !association.polymorphic? && association.klass == parent.class
+          end&.foreign_key&.to_sym
+
+          # Search again in case of polymorphic associations, this time matching on the :has_many side
+          # via the :as option, as well as klass
+          foreign_key ||= parent_class.reflect_on_all_associations(:has_many).find do |has_many_assoc|
+            !matching_parent_child_polymorphic_association(has_many_assoc, child_class).nil?
+          end&.foreign_key&.to_sym
+
           foreign_key.nil? ? nil : all_conditions[foreign_key]
         end
+
+        def matching_parent_child_polymorphic_association(parent_assoc, child_class)
+          return nil unless parent_assoc.klass == child_class
+          return nil if parent_assoc&.options[:as].nil?
+
+          child_class.reflect_on_all_associations(:belongs_to).find do |child_assoc|
+            # Only match this way for polymorphic associations
+            child_assoc.polymorphic? && child_assoc.name == parent_assoc.options[:as]
+          end
+        end
+
+        def child_association_to_parent(parent, child)
+          child_class = child.is_a?(Class) ? child : child.class
+          parent_class = parent.is_a?(Class) ? parent : parent.class
+
+          association = child_class.reflect_on_all_associations(:belongs_to).find do |association|
+            # Do not match on polymorphic associations or it will throw an error (klass cannot be determined)
+            !association.polymorphic? && association.klass == parent.class
+          end
+
+          return association unless association.nil?
+
+          parent_class.reflect_on_all_associations(:has_many).each do |has_many_assoc|
+            association ||= matching_parent_child_polymorphic_association(has_many_assoc, child_class)
+          end
+
+          association
+        end
+
+        def parent_condition_name(parent, child)
+          child_association_to_parent(parent, child)&.name || parent.class.name.downcase.to_sym
+        end
       end
 
       # Returns conditions intended to be used inside a database query. Normally you will not call this
@@ -133,7 +179,7 @@ def override_scope
       def raise_override_scope_error
         rule_found = @compressed_rules.detect { |rule| rule.conditions.is_a?(ActiveRecord::Relation) }
         raise Error,
-              'Unable to merge an Active Record scope with other conditions. '\
+              'Unable to merge an Active Record scope with other conditions. ' \
               "Instead use a hash or SQL for #{rule_found.actions.first} #{rule_found.subjects.first} ability."
       end
 

lib/cancan/model_adapters/sti_normalizer.rb

@@ -30,8 +30,16 @@ def update_rule(subject, rule, rules_cache)
 
         # create a new rule for the subclasses that links on the inheritance_column
         def build_rule_for_subclass(rule, subject)
+          sti_conditions = { subject.inheritance_column => subject.sti_name }
+          new_rule_conditions =
+            if rule.with_scope?
+              rule.conditions.where(sti_conditions)
+            else
+              rule.conditions.merge(sti_conditions)
+            end
+
           CanCan::Rule.new(rule.base_behavior, rule.actions, subject.superclass,
-                           rule.conditions.merge(subject.inheritance_column => subject.sti_name), rule.block)
+                           new_rule_conditions, rule.block)
         end
       end
     end

lib/cancan/rule.rb

@@ -123,7 +123,7 @@ def parse_attributes_from_extra_args(args)
     def condition_and_block_check(conditions, block, action, subject)
       return unless conditions.is_a?(Hash) && block
 
-      raise BlockAndConditionsError, 'A hash of conditions is mutually exclusive with a block. '\
+      raise BlockAndConditionsError, 'A hash of conditions is mutually exclusive with a block. ' \
         "Check \":#{action} #{subject}\" ability."
     end
 

lib/cancan/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module CanCan
-  VERSION = '3.4.0'.freeze
+  VERSION = '3.5.0'.freeze
 end

spec/cancan/ability_spec.rb

@@ -127,6 +127,17 @@
     expect(@block_called).to be(true)
   end
 
+  it 'allows passing nil as extra arguments' do
+    @ability.can :to_s, Integer do |integer, arg1, arg2|
+      expect(integer).to eq(42)
+      expect(arg1).to eq(nil)
+      expect(arg2).to eq(:foo)
+      @block_called = true
+    end
+    @ability.can?(:to_s, 42, nil, nil, :foo)
+    expect(@block_called).to be(true)
+  end
+
   it 'passes nil to object when comparing class with can check' do
     @ability.can do |action, object_class, object|
       expect(action).to eq(:foo)