Commit bba63bb2 authored by Lizzy's avatar Lizzy Committed by Alessandro Rodi
Browse files

Fixes SQL exception from rails 5.2 upgrade (#479)

Add support for Rails 5.2
parent 3a3306cf
......@@ -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:
......
......@@ -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
......@@ -2,6 +2,7 @@ Unreleased
* Feature cancancan#172 - Include conditions passed to authorize! in AccessDenied exception (kraflab)
* 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)
......
# 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: "../"
......@@ -12,4 +12,5 @@ require 'cancan/model_adapters/default_adapter'
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
......@@ -3,7 +3,7 @@ module CanCan
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 @@ module CanCan
# 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 @@ module CanCan
@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
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
......@@ -39,7 +39,7 @@ if defined? CanCan::ModelAdapters::ActiveRecord4Adapter
.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 @@ if defined? CanCan::ModelAdapters::ActiveRecord4Adapter
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 @@ if defined? CanCan::ModelAdapters::ActiveRecord4Adapter
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])
......
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
......@@ -81,16 +81,17 @@ if defined? CanCan::ModelAdapters::ActiveRecordAdapter
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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment