From 87375fc3dd8193672ee95e498b7c5982ddb7c19d Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 2 Mar 2017 14:49:57 -0700 Subject: [PATCH] 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 Product-Review: Cody Cutrer QA-Review: Cody Cutrer --- config/initializers/postgresql_adapter.rb | 44 +++--- .../active_record_quoting_spec.rb | 146 +++++++++--------- spec/initializers/active_record_spec.rb | 6 +- 3 files changed, 101 insertions(+), 95 deletions(-) diff --git a/config/initializers/postgresql_adapter.rb b/config/initializers/postgresql_adapter.rb index 61c82037319..b2eb8f4132e 100644 --- a/config/initializers/postgresql_adapter.rb +++ b/config/initializers/postgresql_adapter.rb @@ -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 diff --git a/spec/initializers/active_record_quoting_spec.rb b/spec/initializers/active_record_quoting_spec.rb index d87b2f9f3fb..ce967a2689f 100644 --- a/spec/initializers/active_record_quoting_spec.rb +++ b/spec/initializers/active_record_quoting_spec.rb @@ -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 = "" + 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 = "" - 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 diff --git a/spec/initializers/active_record_spec.rb b/spec/initializers/active_record_spec.rb index 53e797dde6e..90a515eb93d 100644 --- a/spec/initializers/active_record_spec.rb +++ b/spec/initializers/active_record_spec.rb @@ -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