Fixes SQL exception from rails 5.2 upgrade

.travis.yml

@@ -12,6 +12,7 @@ gemfile:
  - gemfiles/activerecord_4.2.gemfile
  - gemfiles/activerecord_5.0.2.gemfile
  - gemfiles/activerecord_5.1.0.gemfile
+ - gemfiles/activerecord_5.2.0.gemfile
 services:
  - mongodb
 matrix:
@@ -33,6 +34,10 @@ matrix:
  gemfile: gemfiles/activerecord_5.1.0.gemfile
  - rvm: jruby-9.1.9.0
  gemfile: gemfiles/activerecord_5.1.0.gemfile
+ - rvm: jruby-9.0.5.0
+ gemfile: gemfiles/activerecord_5.2.0.gemfile
+ - rvm: jruby-9.1.9.0
+ gemfile: gemfiles/activerecord_5.2.0.gemfile
 notifications:
  email:
  recipients:

Appraisals

@@ -46,3 +46,19 @@ appraise 'activerecord_5.1.0' do
  gem 'pg', '~> 0.21'
  end
 end
+
+appraise 'activerecord_5.2.0' do
+ gem 'activerecord', '~> 5.2.0', require: 'active_record'
+ gem 'activesupport', '~> 5.2.0', require: 'active_support/all'
+ gem 'actionpack', '~> 5.2.0', require: 'action_pack'
+
+ gemfile.platforms :jruby do
+ gem 'activerecord-jdbcsqlite3-adapter'
+ gem 'jdbc-sqlite3'
+ end
+
+ gemfile.platforms :ruby, :mswin, :mingw do
+ gem 'sqlite3'
+ gem 'pg', '~> 0.21'
+ end
+end

CHANGELOG.rdoc

@@ -1,6 +1,7 @@
 Unreleased
 
 * Removed support for dynamic finders (coorasse)
+* Fix cancancan#478 - Fixes SQL exception from rails 5.2 upgrade (lizzyaustad & kevintyll)
 
 2.1.3 (Jan 16th, 2018)
 

gemfiles/activerecord_5.2.0.gemfile

@@ -0,0 +1,19 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "activerecord", "~> 5.2.0", require: "active_record"
+gem "activesupport", "~> 5.2.0", require: "active_support/all"
+gem "actionpack", "~> 5.2.0", require: "action_pack"
+
+platforms :jruby do
+ gem "activerecord-jdbcsqlite3-adapter"
+ gem "jdbc-sqlite3"
+end
+
+platforms :ruby, :mswin, :mingw do
+ gem "sqlite3"
+ gem "pg", "~> 0.21"
+end
+
+gemspec path: "../"

lib/cancan.rb

@@ -12,4 +12,5 @@
 if defined? ActiveRecord
  require 'cancan/model_adapters/active_record_adapter'
  require 'cancan/model_adapters/active_record_4_adapter'
+ require 'cancan/model_adapters/active_record_5_adapter'
 end

lib/cancan/model_adapters/active_record_4_adapter.rb

@@ -3,7 +3,7 @@ module ModelAdapters
  class ActiveRecord4Adapter < AbstractAdapter
  include ActiveRecordAdapter
  def self.for_class?(model_class)
- model_class <= ActiveRecord::Base
+ ActiveRecord::VERSION::MAJOR == 4 && model_class <= ActiveRecord::Base
  end
 
  # TODO: this should be private
@@ -39,11 +39,8 @@ def build_relation(*where_conditions)
 
  # Rails 4.2 deprecates `sanitize_sql_hash_for_conditions`
  def sanitize_sql(conditions)
- if ActiveRecord::VERSION::MAJOR > 4 && conditions.is_a?(Hash)
- sanitize_sql_activerecord5(conditions)
- elsif ActiveRecord::VERSION::MINOR >= 2 && conditions.is_a?(Hash)
+ if ActiveRecord::VERSION::MINOR >= 2 && conditions.is_a?(Hash)
  sanitize_sql_activerecord4(conditions)
-
  else
  @model_class.send(:sanitize_sql, conditions)
  end
@@ -59,21 +56,6 @@ def sanitize_sql_activerecord4(conditions)
  @model_class.send(:connection).visitor.compile b
  end.join(' AND ')
  end
-
- def sanitize_sql_activerecord5(conditions)
- table = @model_class.send(:arel_table)
- table_metadata = ActiveRecord::TableMetadata.new(@model_class, table)
- predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata)
-
- conditions = predicate_builder.resolve_column_aliases(conditions)
- conditions = @model_class.send(:expand_hash_conditions_for_aggregates, conditions)
-
- conditions.stringify_keys!
-
- predicate_builder.build_from_hash(conditions).map do |b|
- @model_class.send(:connection).visitor.compile b
- end.join(' AND ')
- end
  end
  end
 end

lib/cancan/model_adapters/active_record_5_adapter.rb

@@ -0,0 +1,70 @@
+module CanCan
+ module ModelAdapters
+ class ActiveRecord5Adapter < ActiveRecord4Adapter
+ AbstractAdapter.inherited(self)
+
+ def self.for_class?(model_class)
+ ActiveRecord::VERSION::MAJOR == 5 && model_class <= ActiveRecord::Base
+ end
+
+ # rails 5 is capable of using strings in enum
+ # but often people use symbols in rules
+ def self.matches_condition?(subject, name, value)
+ return super if Array.wrap(value).all? { |x| x.is_a? Integer }
+
+ attribute = subject.send(name)
+ if value.is_a?(Enumerable)
+ value.map(&:to_s).include? attribute
+ else
+ attribute == value.to_s
+ end
+ end
+
+ private
+
+ # As of rails 4, `includes()` no longer causes active record to
+ # look inside the where clause to decide to outer join tables
+ # you're using in the where. Instead, `references()` is required
+ # in addition to `includes()` to force the outer join.
+ def build_relation(*where_conditions)
+ relation = @model_class.where(*where_conditions)
+ relation = relation.includes(joins).references(joins) if joins.present?
+ relation
+ end
+
+ # Rails 4.2 deprecates `sanitize_sql_hash_for_conditions`
+ def sanitize_sql(conditions)
+ if conditions.is_a?(Hash)
+ sanitize_sql_activerecord5(conditions)
+ else
+ @model_class.send(:sanitize_sql, conditions)
+ end
+ end
+
+ def sanitize_sql_activerecord5(conditions)
+ table = @model_class.send(:arel_table)
+ table_metadata = ActiveRecord::TableMetadata.new(@model_class, table)
+ predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata)
+
+ conditions = predicate_builder.resolve_column_aliases(conditions)
+
+ conditions.stringify_keys!
+
+ predicate_builder.build_from_hash(conditions).map do |b|
+ visit_nodes(b)
+ end.join(' AND ')
+ end
+
+ def visit_nodes(b)
+ # Rails 5.2 adds a BindParam node that prevents the visitor method from properly compiling the SQL query
+ if ActiveRecord::VERSION::MINOR >= 2
+ connection = @model_class.send(:connection)
+ collector = Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new)
+ connection.visitor.accept(b, collector).value
+ else
+ @model_class.send(:connection).visitor.compile(b)
+ end
+ end
+ end
+ end
+end

spec/cancan/model_adapters/active_record_4_adapter_spec.rb

@@ -39,7 +39,7 @@ class Child < ActiveRecord::Base
  .to eq [child2, child1]
  end
 
- if ActiveRecord::VERSION::MINOR >= 1
+ if ActiveRecord::VERSION::MINOR >= 1 || ActiveRecord::VERSION::MAJOR >= 5
  it 'allows filters on enums' do
  ActiveRecord::Schema.define do
  create_table(:shapes) do |t|
@@ -48,7 +48,9 @@ class Child < ActiveRecord::Base
  end
 
  class Shape < ActiveRecord::Base
- enum color: %i[red green blue]
+ unless defined_enums.keys.include? 'color'
+ enum color: %i[red green blue]
+ end
  end
 
  red = Shape.create!(color: :red)
@@ -86,8 +88,8 @@ class Shape < ActiveRecord::Base
  end
 
  class Disc < ActiveRecord::Base
- enum color: %i[red green blue]
- enum shape: { triangle: 3, rectangle: 4 }
+ enum color: %i[red green blue] unless defined_enums.keys.include? 'color'
+ enum shape: { triangle: 3, rectangle: 4 } unless defined_enums.keys.include? 'shape'
  end
 
  red_triangle = Disc.create!(color: Disc.colors[:red], shape: Disc.shapes[:triangle])

spec/cancan/model_adapters/active_record_5_adapter_spec.rb

@@ -0,0 +1,100 @@
+require 'spec_helper'
+
+if ActiveRecord::VERSION::MAJOR == 5 && defined?(CanCan::ModelAdapters::ActiveRecord5Adapter)
+ describe CanCan::ModelAdapters::ActiveRecord5Adapter do
+ context 'with sqlite3' do
+ before :each do
+ ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
+ ActiveRecord::Migration.verbose = false
+ ActiveRecord::Schema.define do
+ create_table(:parents) do |t|
+ t.timestamps null: false
+ end
+
+ create_table(:children) do |t|
+ t.timestamps null: false
+ t.integer :parent_id
+ end
+ end
+
+ class Parent < ActiveRecord::Base
+ has_many :children, -> { order(id: :desc) }
+ end
+
+ class Child < ActiveRecord::Base
+ belongs_to :parent
+ end
+
+ (@ability = double).extend(CanCan::Ability)
+ end
+
+ it 'allows filters on enums' do
+ ActiveRecord::Schema.define do
+ create_table(:shapes) do |t|
+ t.integer :color, default: 0, null: false
+ end
+ end
+
+ class Shape < ActiveRecord::Base
+ unless defined_enums.keys.include? 'color'
+ enum color: %i[red green blue]
+ end
+ end
+
+ red = Shape.create!(color: :red)
+ green = Shape.create!(color: :green)
+ blue = Shape.create!(color: :blue)
+
+ # A condition with a single value.
+ @ability.can :read, Shape, color: :green
+
+ expect(@ability.cannot?(:read, red)).to be true
+ expect(@ability.can?(:read, green)).to be true
+ expect(@ability.cannot?(:read, blue)).to be true
+
+ accessible = Shape.accessible_by(@ability)
+ expect(accessible).to contain_exactly(green)
+
+ # A condition with multiple values.
+ @ability.can :update, Shape, color: %i[red blue]
+
+ expect(@ability.can?(:update, red)).to be true
+ expect(@ability.cannot?(:update, green)).to be true
+ expect(@ability.can?(:update, blue)).to be true
+
+ accessible = Shape.accessible_by(@ability, :update)
+ expect(accessible).to contain_exactly(red, blue)
+ end
+
+ it 'allows dual filter on enums' do
+ ActiveRecord::Schema.define do
+ create_table(:discs) do |t|
+ t.integer :color, default: 0, null: false
+ t.integer :shape, default: 3, null: false
+ end
+ end
+
+ class Disc < ActiveRecord::Base
+ enum color: %i[red green blue] unless defined_enums.keys.include? 'color'
+ enum shape: { triangle: 3, rectangle: 4 } unless defined_enums.keys.include? 'shape'
+ end
+
+ red_triangle = Disc.create!(color: :red, shape: :triangle)
+ green_triangle = Disc.create!(color: :green, shape: :triangle)
+ green_rectangle = Disc.create!(color: :green, shape: :rectangle)
+ blue_rectangle = Disc.create!(color: :blue, shape: :rectangle)
+
+ # A condition with a dual filter.
+ @ability.can :read, Disc, color: :green, shape: :rectangle
+
+ expect(@ability.cannot?(:read, red_triangle)).to be true
+ expect(@ability.cannot?(:read, green_triangle)).to be true
+ expect(@ability.can?(:read, green_rectangle)).to be true
+ expect(@ability.cannot?(:read, blue_rectangle)).to be true
+
+ accessible = Disc.accessible_by(@ability)
+ expect(accessible).to contain_exactly(green_rectangle)
+ end
+ end
+ end
+end

spec/cancan/model_adapters/active_record_adapter_spec.rb

@@ -81,16 +81,17 @@ class User < ActiveRecord::Base
 
  it 'is for only active record classes' do
  if ActiveRecord.respond_to?(:version) &&
- ActiveRecord.version > Gem::Version.new('4')
+ ActiveRecord.version > Gem::Version.new('5')
+ expect(CanCan::ModelAdapters::ActiveRecord5Adapter).to_not be_for_class(Object)
+ expect(CanCan::ModelAdapters::ActiveRecord5Adapter).to be_for_class(Article)
+ expect(CanCan::ModelAdapters::AbstractAdapter.adapter_class(Article))
+ .to eq(CanCan::ModelAdapters::ActiveRecord5Adapter)
+ elsif ActiveRecord.respond_to?(:version) &&
+ ActiveRecord.version > Gem::Version.new('4')
  expect(CanCan::ModelAdapters::ActiveRecord4Adapter).to_not be_for_class(Object)
  expect(CanCan::ModelAdapters::ActiveRecord4Adapter).to be_for_class(Article)
  expect(CanCan::ModelAdapters::AbstractAdapter.adapter_class(Article))
  .to eq(CanCan::ModelAdapters::ActiveRecord4Adapter)
- else
- expect(CanCan::ModelAdapters::ActiveRecord3Adapter).to_not be_for_class(Object)
- expect(CanCan::ModelAdapters::ActiveRecord3Adapter).to be_for_class(Article)
- expect(CanCan::ModelAdapters::AbstractAdapter.adapter_class(Article))
- .to eq(CanCan::ModelAdapters::ActiveRecord3Adapter)
  end
  end