ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

rails 5: fix initializer specs 

in particular, completely dump the quoting monkeypatch and specs -
we no longer know what column it's quoting for at this level, and
the float problems have long been fixed in rails proper

Change-Id: I9bbd214e6c866fcae1222f368f544f4805aac1cf
Reviewed-on: https://gerrit.instructure.com/103830
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2017-03-02 14:49:57 -07:00
parent d2d7b81fee
commit 87375fc3dd
3 changed files with 101 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
config/initializers/postgresql_adapter.rb
View File

@ -177,30 +177,32 @@ module PostgreSQLAdapterExtensions
end
end
# Force things with (approximate) integer representations (Floats,
# BigDecimals, Times, etc.) into those representations. Raise
# ActiveRecord::StatementInvalid for any other non-integer things.
def quote(value, column = nil)
return value if value.is_a?(QuotedValue)
if CANVAS_RAILS4_2
# Force things with (approximate) integer representations (Floats,
# BigDecimals, Times, etc.) into those representations. Raise
# ActiveRecord::StatementInvalid for any other non-integer things.
def quote(value, column = nil)
return value if value.is_a?(QuotedValue)
if column && column.type == :integer && !value.respond_to?(:quoted_id)
case value
when String, ActiveSupport::Multibyte::Chars, nil, true, false
# these already have branches for column.type == :integer (or don't
# need one)
super(value, column)
else
if value.respond_to?(:to_i)
# quote the value in its integer representation
value.to_i.to_s
if column && column.type == :integer && !value.respond_to?(:quoted_id)
case value
when String, ActiveSupport::Multibyte::Chars, nil, true, false
# these already have branches for column.type == :integer (or don't
# need one)
super(value, column)
else
# doesn't have a (known) integer representation, can't quote it
# for an integer column
raise ActiveRecord::StatementInvalid, "#{value.inspect} cannot be interpreted as an integer"
end
if value.respond_to?(:to_i)
# quote the value in its integer representation
value.to_i.to_s
else
# doesn't have a (known) integer representation, can't quote it
# for an integer column
raise ActiveRecord::StatementInvalid, "#{value.inspect} cannot be interpreted as an integer"
end
end
else
super
end
else
super
end
end

146
spec/initializers/active_record_quoting_spec.rb
View File

@ -1,88 +1,90 @@
require File.expand_path('../spec_helper', File.dirname( __FILE__ ))
module ActiveRecord
module ConnectionAdapters
if defined?(PostgreSQLAdapter)
describe PostgreSQLAdapter do
describe 'quoting' do
# These tests are adapted from the ActiveRecord tests located here:
# https://github.com/rails/rails/blob/06c23c4c7ff842f7c6237f3ac43fc9d19509a947/activerecord/test/cases/adapters/postgresql/quoting_test.rb
before do
@conn = ActiveRecord::Base.connection
end
def column(type)
sql_type = @conn.type_to_sql(type)
PostgreSQLColumn.new(nil, 1, @conn.lookup_cast_type(sql_type), sql_type)
end
describe "Infinity and NaN" do
it 'properly quotes NaN' do
nan = 0.0/0
c = column('float')
assert_equal "'NaN'", @conn.quote(nan, c)
end
it 'properly quotes Infinity' do
infinity = 1.0/0
c = column('float')
assert_equal "'Infinity'", @conn.quote(infinity, c)
end
it 'properly quotes Infinity in a datetime column' do
infinity = 1.0/0
c = column('datetime')
assert_equal ("'Infinity'"), @conn.quote(infinity, c)
end
end
describe "integer enforcement" do
if CANVAS_RAILS4_2
module ActiveRecord
module ConnectionAdapters
if defined?(PostgreSQLAdapter)
describe PostgreSQLAdapter do
describe 'quoting' do
# These tests are adapted from the ActiveRecord tests located here:
# https://github.com/rails/rails/blob/06c23c4c7ff842f7c6237f3ac43fc9d19509a947/activerecord/test/cases/adapters/postgresql/quoting_test.rb
before do
@col = column('integer')
@conn = ActiveRecord::Base.connection
end
it 'properly quotes Numerics' do
assert_equal "123", @conn.quote(123, @col)
assert_equal "100000000000000000000", @conn.quote(1e20.to_i, @col)
assert_equal "1", @conn.quote(1.23, @col)
assert_equal "1", @conn.quote(BigDecimal.new("1.23"), @col)
def column(type)
sql_type = @conn.type_to_sql(type)
PostgreSQLColumn.new(nil, 1, @conn.lookup_cast_type(sql_type), sql_type)
end
it 'properly quotes numeric Strings' do
assert_equal "123", @conn.quote("123", @col)
assert_equal "1", @conn.quote("1.23", @col)
describe "Infinity and NaN" do
it 'properly quotes NaN' do
nan = 0.0/0
c = column('float')
assert_equal "'NaN'", @conn.quote(nan, c)
end
it 'properly quotes Infinity' do
infinity = 1.0/0
c = column('float')
assert_equal "'Infinity'", @conn.quote(infinity, c)
end
it 'properly quotes Infinity in a datetime column' do
infinity = 1.0/0
c = column('datetime')
assert_equal ("'Infinity'"), @conn.quote(infinity, c)
end
end
it 'properly quotes Times' do
value = Time.at(1356998400) # Jan 1, 2013 00:00 GMT
assert_equal "1356998400", @conn.quote(value, @col)
describe "integer enforcement" do
before do
@col = column('integer')
end
it 'properly quotes Numerics' do
assert_equal "123", @conn.quote(123, @col)
assert_equal "100000000000000000000", @conn.quote(1e20.to_i, @col)
assert_equal "1", @conn.quote(1.23, @col)
assert_equal "1", @conn.quote(BigDecimal.new("1.23"), @col)
end
it 'properly quotes numeric Strings' do
assert_equal "123", @conn.quote("123", @col)
assert_equal "1", @conn.quote("1.23", @col)
end
it 'properly quotes Times' do
value = Time.at(1356998400) # Jan 1, 2013 00:00 GMT
assert_equal "1356998400", @conn.quote(value, @col)
end
it 'aborts quoting non-Numerics' do
expect{ @conn.quote(:symbol, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
expect{ @conn.quote(ActiveRecord::Base, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
expect{ @conn.quote(Object.new, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
end
end
it 'aborts quoting non-Numerics' do
expect{ @conn.quote(:symbol, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
expect{ @conn.quote(ActiveRecord::Base, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
expect{ @conn.quote(Object.new, @col) }.to raise_exception(ActiveRecord::StatementInvalid)
end
end
# check we didn't screw up default handlings
describe "fallback to original implementation" do
it 'properly quotes strings in xml columns' do
value = "<value/>"
c = column('xml')
assert_equal "xml '#{value}'", @conn.quote(value, c)
end
# check we didn't screw up default handlings
describe "fallback to original implementation" do
it 'properly quotes strings in xml columns' do
value = "<value/>"
c = column('xml')
assert_equal "xml '#{value}'", @conn.quote(value, c)
end
it 'properly quotes other Floats' do
value = 1.23
c = column('float')
assert_equal value.to_s, @conn.quote(value, c)
end
it 'properly quotes other Floats' do
value = 1.23
c = column('float')
assert_equal value.to_s, @conn.quote(value, c)
end
it 'properly quotes other non-Numerics' do
value = "value"
c = column('string')
assert_equal "'#{value}'", @conn.quote(value, c)
it 'properly quotes other non-Numerics' do
value = "value"
c = column('string')
assert_equal "'#{value}'", @conn.quote(value, c)
end
end
end
end

6
spec/initializers/active_record_spec.rb
View File

@ -242,7 +242,8 @@ module ActiveRecord
describe "union" do
shared_examples_for "query creation" do
it "should include conditions after the union inside of the subquery" do
wheres = base.active.where(id:99).union(User.where(id:1)).where_values
scope = base.active.where(id:99).union(User.where(id:1))
wheres = CANVAS_RAILS4_2 ? scope.where_values : scope.where_clause.send(:predicates)
expect(wheres.count).to eq 1
sql_before_union, sql_after_union = wheres.first.split("UNION ALL")
expect(sql_before_union.include?('"id" = 99')).to be_falsey
@ -250,7 +251,8 @@ module ActiveRecord
end
it "should include conditions prior to the union outside of the subquery" do
wheres = base.active.union(User.where(id:1)).where(id:99).where_values
scope = base.active.union(User.where(id:1)).where(id:99)
wheres = CANVAS_RAILS4_2 ? scope.where_values : scope.where_clause.send(:predicates)
expect(wheres.count).to eq 2
union_where = wheres.detect{|w| w.is_a?(String) && w.include?("UNION ALL")}
expect(union_where.include?('"id" = 99')).to be_falsey