accessible_by_strategy only works for Rails 5+

.byebug_history

@@ -0,0 +1,4 @@
+exit
+c
+valid_accessible_by_strategies
+value

lib/cancan/config.rb

@@ -1,10 +1,11 @@
 # frozen_string_literal: true
 
 module CanCan
-  VALID_ACCESSIBLE_BY_STRATEGIES = %i[
-    subquery
-    left_join
-  ].freeze
+  def self.valid_accessible_by_strategies
+    strategies = [:left_join]
+    strategies << :subquery unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+    strategies
+  end
 
   # Determines how CanCan should build queries when calling accessible_by,
   # if the query will contain a join. The default strategy is `:subquery`.
@@ -20,12 +21,27 @@ module CanCan
   #                `distinct` is not reliable in some cases. See
   #                https://github.com/CanCanCommunity/cancancan/pull/605
   def self.accessible_by_strategy
-    @accessible_by_strategy || :subquery
+    @accessible_by_strategy || default_accessible_by_strategy
+  end
+
+  def self.default_accessible_by_strategy
+    if CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+      # see https://github.com/CanCanCommunity/cancancan/pull/655 for where this was added
+      # the `subquery` strategy (from https://github.com/CanCanCommunity/cancancan/pull/619
+      # only works in Rails 5 and higher
+      :left_join
+    else
+      :subquery
+    end
   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(', ')}"
+    unless valid_accessible_by_strategies.include?(value)
+      raise ArgumentError, "accessible_by_strategy must be one of #{valid_accessible_by_strategies.join(', ')}"
+    end
+
+    if value == :subquery && CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+      raise ArgumentError, 'accessible_by_strategy = :subquery requires ActiveRecord 5 or newer'
     end
 
     @accessible_by_strategy = value

lib/cancan/model_adapters/active_record_4_adapter.rb

@@ -35,14 +35,7 @@ def matches_condition?(subject, name, value)
       # you're using in the where. Instead, `references()` is required
       # in addition to `includes()` to force the outer join.
       def build_joins_relation(relation, *_where_conditions)
-        case CanCan.accessible_by_strategy
-        when :subquery
-          # subquery mode doesn't work with Rails 4.x
-          relation.includes(joins).references(joins)
-
-        when :left_join
-          relation.includes(joins).references(joins)
-        end
+        relation.includes(joins).references(joins)
       end
 
       # Rails 4.2 deprecates `sanitize_sql_hash_for_conditions`

spec/cancan/model_adapters/accessible_by_has_many_through_spec.rb

@@ -72,7 +72,7 @@ class Editor < ActiveRecord::Base
     ability.can :read, Post, editors: { user_id: @user1 }
   end
 
-  CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+  CanCan.valid_accessible_by_strategies.each do |strategy|
     context "using #{strategy} strategy" do
       before :each do
         CanCan.accessible_by_strategy = strategy
@@ -104,14 +104,16 @@ class Editor < ActiveRecord::Base
     end
   end
 
-  describe 'filtering of results - subquery' do
-    before :each do
-      CanCan.accessible_by_strategy = :subquery
-    end
+  unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+    describe 'filtering of results - subquery' do
+      before :each do
+        CanCan.accessible_by_strategy = :subquery
+      end
 
-    it 'adds the where clause correctly with joins' do
-      posts = Post.joins(:editors).where('editors.user_id': @user1.id).accessible_by(ability)
-      expect(posts.length).to eq 1
+      it 'adds the where clause correctly with joins' do
+        posts = Post.joins(:editors).where('editors.user_id': @user1.id).accessible_by(ability)
+        expect(posts.length).to eq 1
+      end
     end
   end
 

spec/cancan/model_adapters/active_record_4_adapter_spec.rb

@@ -4,7 +4,8 @@
 
 if CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
   describe CanCan::ModelAdapters::ActiveRecord4Adapter do
-    CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+    # only the `left_join` strategy works in AR4
+    CanCan.valid_accessible_by_strategies.each do |strategy|
       context "with sqlite3 and #{strategy} strategy" do
         before :each do
           CanCan.accessible_by_strategy = strategy

spec/cancan/model_adapters/active_record_5_adapter_spec.rb

@@ -4,7 +4,7 @@
 
 if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
   describe CanCan::ModelAdapters::ActiveRecord5Adapter do
-    CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+    CanCan.valid_accessible_by_strategies.each do |strategy|
       context "with sqlite3 and #{strategy} strategy" do
         before :each do
           CanCan.accessible_by_strategy = strategy

spec/cancan/model_adapters/active_record_adapter_spec.rb

@@ -109,7 +109,7 @@ class User < ActiveRecord::Base
     @comment_table = Comment.table_name
   end
 
-  CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+  CanCan.valid_accessible_by_strategies.each do |strategy|
     context "base functionality with #{strategy} strategy" do
       before :each do
         CanCan.accessible_by_strategy = strategy
@@ -444,30 +444,32 @@ class User < ActiveRecord::Base
     end
   end
 
-  context 'base behaviour subquery specific' do
-    before :each do
-      CanCan.accessible_by_strategy = :subquery
-    end
+  unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+    context 'base behaviour subquery specific' do
+      before :each do
+        CanCan.accessible_by_strategy = :subquery
+      end
 
-    it 'allows ordering via relations' do
-      @ability.can :read, Comment, article: { category: { visible: true } }
-      comment1 = Comment.create!(article: Article.create!(name: 'B', category: Category.create!(visible: true)))
-      comment2 = Comment.create!(article: Article.create!(name: 'A', category: Category.create!(visible: true)))
-      Comment.create!(article: Article.create!(category: Category.create!(visible: false)))
+      it 'allows ordering via relations' do
+        @ability.can :read, Comment, article: { category: { visible: true } }
+        comment1 = Comment.create!(article: Article.create!(name: 'B', category: Category.create!(visible: true)))
+        comment2 = Comment.create!(article: Article.create!(name: 'A', category: Category.create!(visible: true)))
+        Comment.create!(article: Article.create!(category: Category.create!(visible: false)))
 
-      # doesn't work without explicitly calling a join on AR 5+,
-      # but does before that (where we don't use subqueries at all)
-      if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
-        expect { Comment.accessible_by(@ability).order('articles.name').to_a }
-          .to raise_error(ActiveRecord::StatementInvalid)
-      else
-        expect(Comment.accessible_by(@ability).order('articles.name'))
+        # doesn't work without explicitly calling a join on AR 5+,
+        # but does before that (where we don't use subqueries at all)
+        if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
+          expect { Comment.accessible_by(@ability).order('articles.name').to_a }
+            .to raise_error(ActiveRecord::StatementInvalid)
+        else
+          expect(Comment.accessible_by(@ability).order('articles.name'))
+            .to match_array([comment2, comment1])
+        end
+
+        # works with the explicit join
+        expect(Comment.accessible_by(@ability).joins(:article).order('articles.name'))
           .to match_array([comment2, comment1])
       end
-
-      # works with the explicit join
-      expect(Comment.accessible_by(@ability).joins(:article).order('articles.name'))
-        .to match_array([comment2, comment1])
     end
   end
 
@@ -612,7 +614,7 @@ class Transaction < ActiveRecord::Base
     end
   end
 
-  CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+  CanCan.valid_accessible_by_strategies.each do |strategy|
     context "when a table is referenced multiple times with #{strategy} strategy" do
       before :each do
         CanCan.accessible_by_strategy = strategy
@@ -635,33 +637,35 @@ class Transaction < ActiveRecord::Base
     end
   end
 
-  context 'has_many through is defined and referenced differently - subquery strategy' do
-    before do
-      CanCan.accessible_by_strategy = :subquery
-    end
+  unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+    context 'has_many through is defined and referenced differently - subquery strategy' do
+      before do
+        CanCan.accessible_by_strategy = :subquery
+      end
 
-    it 'recognises it and simplifies the query' do
-      u1 = User.create!(name: 'pippo')
-      u2 = User.create!(name: 'paperino')
+      it 'recognises it and simplifies the query' do
+        u1 = User.create!(name: 'pippo')
+        u2 = User.create!(name: 'paperino')
 
-      a1 = Article.create!(mentioned_users: [u1])
-      a2 = Article.create!(mentioned_users: [u2])
+        a1 = Article.create!(mentioned_users: [u1])
+        a2 = Article.create!(mentioned_users: [u2])
 
-      ability = Ability.new(u1)
-      ability.can :read, Article, mentioned_users: { name: u1.name }
-      ability.can :read, Article, mentions: { user: { name: u2.name } }
-      expect(Article.accessible_by(ability)).to match_array([a1, a2])
-      if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
-        expect(ability.model_adapter(Article, :read)).to generate_sql(%(
-  SELECT "articles".*
-  FROM "articles"
-  WHERE "articles"."id" IN
-  (SELECT "articles"."id"
+        ability = Ability.new(u1)
+        ability.can :read, Article, mentioned_users: { name: u1.name }
+        ability.can :read, Article, mentions: { user: { name: u2.name } }
+        expect(Article.accessible_by(ability)).to match_array([a1, a2])
+        if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
+          expect(ability.model_adapter(Article, :read)).to generate_sql(%(
+    SELECT "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" = 'paperino') OR ("users"."name" = 'pippo')))
-  ))
+    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" = 'paperino') OR ("users"."name" = 'pippo')))
+    ))
+        end
       end
     end
   end
@@ -694,7 +698,7 @@ class Transaction < ActiveRecord::Base
     end
   end
 
-  CanCan::VALID_ACCESSIBLE_BY_STRATEGIES.each do |strategy|
+  CanCan.valid_accessible_by_strategies.each do |strategy|
     context "when a model has renamed primary_key with #{strategy} strategy" do
       before :each do
         CanCan.accessible_by_strategy = strategy

spec/cancan/model_adapters/has_and_belongs_to_many_spec.rb

@@ -45,28 +45,30 @@ class House < ActiveRecord::Base
     ability.can :read, House, people: { id: @person1.id }
   end
 
-  describe 'fetching of records - subquery strategy' do
-    before do
-      CanCan.accessible_by_strategy = :subquery
-    end
+  unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
+    describe 'fetching of records - subquery strategy' do
+      before do
+        CanCan.accessible_by_strategy = :subquery
+      end
 
-    it 'it retreives the records correctly' do
-      houses = House.accessible_by(ability)
-      expect(houses).to match_array [@house2, @house1]
-    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\"
-                          WHERE \"houses\".\"id\" IN
-                            (SELECT \"houses\".\"id\"
+      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\"
-                            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})
-                          ")
+                            WHERE \"houses\".\"id\" IN
+                              (SELECT \"houses\".\"id\"
+                              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})
+                            ")
+        end
       end
     end
   end

spec/spec_helper.rb

@@ -29,8 +29,7 @@
   config.include SQLHelpers
 
   config.after :each do
-    # set default values for all config
-    CanCan.accessible_by_strategy = :subquery
+    CanCan.accessible_by_strategy = CanCan.default_accessible_by_strategy
   end
 end