From ef0ea782b1f5cf7b08e74ea3002a16c708f66645 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 31 May 2008 16:57:46 -0700 Subject: Added SQL escaping for :limit and :offset [#288 state:closed] (Aaron Bedra, Steven Bristol, Jonathan Wiess) --- .../abstract/database_statements.rb | 9 +++++++-- activerecord/test/cases/adapter_test.rb | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 16d405d3bd..5358491cde 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -106,11 +106,16 @@ module ActiveRecord # SELECT * FROM suppliers LIMIT 10 OFFSET 50 def add_limit_offset!(sql, options) if limit = options[:limit] - sql << " LIMIT #{limit}" + sql << " LIMIT #{sanitize_limit(limit)}" if offset = options[:offset] - sql << " OFFSET #{offset}" + sql << " OFFSET #{offset.to_i}" end end + sql + end + + def sanitize_limit(limit) + limit.to_s[/,/] ? limit.split(',').map{ |i| i.to_i }.join(',') : limit.to_i end # Appends a locking clause to an SQL statement. diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 91504af901..c77446f880 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -104,4 +104,24 @@ class AdapterTest < ActiveRecord::TestCase end end + def test_add_limit_offset_should_sanitize_sql_injection_for_limit_without_comas + sql_inject = "1 select * from schema" + assert_equal " LIMIT 1", @connection.add_limit_offset!("", :limit=>sql_inject) + if current_adapter?(:MysqlAdapter) + assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + else + assert_equal " LIMIT 1 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + end + end + + def test_add_limit_offset_should_sanitize_sql_injection_for_limit_with_comas + sql_inject = "1, 7 procedure help()" + if current_adapter?(:MysqlAdapter) + assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) + assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + else + assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) + assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + end + end end -- cgit v1.2.3 From f6e921f9568d7f2e4807edf8728e6b0df8991816 Mon Sep 17 00:00:00 2001 From: "John D. Hume" Date: Wed, 28 May 2008 23:35:56 -0400 Subject: Substitute value into validates_format_of message Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/validations.rb | 2 +- activerecord/test/cases/validations_test.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 0e150bddb2..c97aafb126 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -692,7 +692,7 @@ module ActiveRecord raise(ArgumentError, "A regular expression must be supplied as the :with option of the configuration hash") unless configuration[:with].is_a?(Regexp) validates_each(attr_names, configuration) do |record, attr_name, value| - record.errors.add(attr_name, configuration[:message]) unless value.to_s =~ configuration[:with] + record.errors.add(attr_name, configuration[:message] % value) unless value.to_s =~ configuration[:with] end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index a4d9da4806..7b71647d25 100755 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -583,6 +583,12 @@ class ValidationsTest < ActiveRecord::TestCase assert_nil t.errors.on(:title) end + def test_validate_format_with_formatted_message + Topic.validates_format_of(:title, :with => /^Valid Title$/, :message => "can't be %s") + t = Topic.create(:title => 'Invalid title') + assert_equal "can't be Invalid title", t.errors.on(:title) + end + def test_validates_inclusion_of Topic.validates_inclusion_of( :title, :in => %w( a b c d e f g ) ) -- cgit v1.2.3 From f9db7695fe3c148c8d1077f1564e5b94d126b83b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 31 May 2008 17:03:03 -0700 Subject: Making ready for release of 2.1 --- activerecord/CHANGELOG | 6 ++++-- activerecord/Rakefile | 2 +- activerecord/lib/active_record/version.rb | 4 ++-- activerecord/test/connections/native_mysql/connection.rb | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 78082665a4..f901e432d5 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,6 +1,8 @@ -* Add first/last methods to associations/named_scope. Resolved #226. [Ryan Bates] +*2.1.0 (May 31st, 2008)* -*2.1.0 RC1 (May 11th, 2008)* +* Added SQL escaping for :limit and :offset #288 [Aaron Bedra, Steven Bristol, Jonathan Wiess] + +* Added first/last methods to associations/named_scope. Resolved #226. [Ryan Bates] * Ensure hm:t preloading honours reflection options. Resolves #137. [Frederick Cheung] diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 043ab6d551..fc068b16c3 100755 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -171,7 +171,7 @@ spec = Gem::Specification.new do |s| s.files = s.files + Dir.glob( "#{dir}/**/*" ).delete_if { |item| item.include?( "\.svn" ) } end - s.add_dependency('activesupport', '= 2.0.991' + PKG_BUILD) + s.add_dependency('activesupport', '= 2.1.0' + PKG_BUILD) s.files.delete FIXTURES_ROOT + "/fixture_database.sqlite" s.files.delete FIXTURES_ROOT + "/fixture_database_2.sqlite" diff --git a/activerecord/lib/active_record/version.rb b/activerecord/lib/active_record/version.rb index 1463e84764..aaadef9979 100644 --- a/activerecord/lib/active_record/version.rb +++ b/activerecord/lib/active_record/version.rb @@ -1,8 +1,8 @@ module ActiveRecord module VERSION #:nodoc: MAJOR = 2 - MINOR = 0 - TINY = 991 + MINOR = 1 + TINY = 0 STRING = [MAJOR, MINOR, TINY].join('.') end diff --git a/activerecord/test/connections/native_mysql/connection.rb b/activerecord/test/connections/native_mysql/connection.rb index 1fab444e58..3ee031f384 100644 --- a/activerecord/test/connections/native_mysql/connection.rb +++ b/activerecord/test/connections/native_mysql/connection.rb @@ -12,13 +12,13 @@ ActiveRecord::Base.logger = RAILS_DEFAULT_LOGGER ActiveRecord::Base.configurations = { 'arunit' => { :adapter => 'mysql', - :username => 'rails', + :username => 'root', :encoding => 'utf8', :database => 'activerecord_unittest', }, 'arunit2' => { :adapter => 'mysql', - :username => 'rails', + :username => 'root', :database => 'activerecord_unittest2' } } -- cgit v1.2.3 From ea03b0885c110003496c1f99dc7d9d2f1534955b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 31 May 2008 17:07:44 -0700 Subject: revert mysql test credential change --- activerecord/test/connections/native_mysql/connection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/connections/native_mysql/connection.rb b/activerecord/test/connections/native_mysql/connection.rb index 3ee031f384..1fab444e58 100644 --- a/activerecord/test/connections/native_mysql/connection.rb +++ b/activerecord/test/connections/native_mysql/connection.rb @@ -12,13 +12,13 @@ ActiveRecord::Base.logger = RAILS_DEFAULT_LOGGER ActiveRecord::Base.configurations = { 'arunit' => { :adapter => 'mysql', - :username => 'root', + :username => 'rails', :encoding => 'utf8', :database => 'activerecord_unittest', }, 'arunit2' => { :adapter => 'mysql', - :username => 'root', + :username => 'rails', :database => 'activerecord_unittest2' } } -- cgit v1.2.3 From 72483c0d4c1e4ea794919974100acc2f255f6fd2 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 31 May 2008 17:13:11 -0700 Subject: Add ActiveRecord::Base.sti_name that checks ActiveRecord::Base#store_full_sti_class? and returns either the full or demodulized name. [rick] [#114 state:resolved] --- activerecord/CHANGELOG | 2 ++ .../associations/has_many_through_association.rb | 2 +- activerecord/lib/active_record/base.rb | 16 ++++++++++++---- activerecord/test/cases/inheritance_test.rb | 18 +++++++++++++++++- 4 files changed, 32 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 78082665a4..31e1c22f0d 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,3 +1,5 @@ +* Add ActiveRecord::Base.sti_name that checks ActiveRecord::Base#store_full_sti_class? and returns either the full or demodulized name. [rick] + * Add first/last methods to associations/named_scope. Resolved #226. [Ryan Bates] *2.1.0 RC1 (May 11th, 2008)* diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index ebcf462f2e..52ced36d16 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -237,7 +237,7 @@ module ActiveRecord end def build_sti_condition - "#{@reflection.through_reflection.quoted_table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.name.demodulize)}" + "#{@reflection.through_reflection.quoted_table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.sti_name)}" end alias_method :sql_conditions, :conditions diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index d2d9bda6ba..92a24ecc5f 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1293,6 +1293,10 @@ module ActiveRecord #:nodoc: super end + def sti_name + store_full_sti_class ? name : name.demodulize + end + private def find_initial(options) options.update(:limit => 1) @@ -1452,7 +1456,11 @@ module ActiveRecord #:nodoc: # Nest the type name in the same module as this class. # Bar is "MyApp::Business::Bar" relative to MyApp::Business::Foo def type_name_with_module(type_name) - (/^::/ =~ type_name) ? type_name : "#{parent.name}::#{type_name}" + if store_full_sti_class + type_name + else + (/^::/ =~ type_name) ? type_name : "#{parent.name}::#{type_name}" + end end def construct_finder_sql(options) @@ -1571,8 +1579,8 @@ module ActiveRecord #:nodoc: def type_condition quoted_inheritance_column = connection.quote_column_name(inheritance_column) - type_condition = subclasses.inject("#{quoted_table_name}.#{quoted_inheritance_column} = '#{store_full_sti_class ? name : name.demodulize}' ") do |condition, subclass| - condition << "OR #{quoted_table_name}.#{quoted_inheritance_column} = '#{store_full_sti_class ? subclass.name : subclass.name.demodulize}' " + type_condition = subclasses.inject("#{quoted_table_name}.#{quoted_inheritance_column} = '#{sti_name}' ") do |condition, subclass| + condition << "OR #{quoted_table_name}.#{quoted_inheritance_column} = '#{subclass.sti_name}' " end " (#{type_condition}) " @@ -2508,7 +2516,7 @@ module ActiveRecord #:nodoc: # Message class in that example. def ensure_proper_type unless self.class.descends_from_active_record? - write_attribute(self.class.inheritance_column, store_full_sti_class ? self.class.name : self.class.name.demodulize) + write_attribute(self.class.inheritance_column, self.class.sti_name) end end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 27394924a1..f09b617e6d 100755 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -5,7 +5,23 @@ require 'models/subscriber' class InheritanceTest < ActiveRecord::TestCase fixtures :companies, :projects, :subscribers, :accounts - + + def test_class_with_store_full_sti_class_returns_full_name + old = ActiveRecord::Base.store_full_sti_class + ActiveRecord::Base.store_full_sti_class = true + assert_equal 'Namespaced::Company', Namespaced::Company.sti_name + ensure + ActiveRecord::Base.store_full_sti_class = old + end + + def test_class_without_store_full_sti_class_returns_demodulized_name + old = ActiveRecord::Base.store_full_sti_class + ActiveRecord::Base.store_full_sti_class = false + assert_equal 'Company', Namespaced::Company.sti_name + ensure + ActiveRecord::Base.store_full_sti_class = old + end + def test_should_store_demodulized_class_name_with_store_full_sti_class_option_disabled old = ActiveRecord::Base.store_full_sti_class ActiveRecord::Base.store_full_sti_class = false -- cgit v1.2.3 From 3282bf3b5016f0c9028cfff1012e8c31a13b40b7 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 1 Jun 2008 09:15:11 -0700 Subject: Added SQL escaping for :limit and :offset in MySQL [Jonathan Wiess] --- activerecord/CHANGELOG | 5 +++++ activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 3 ++- activerecord/test/cases/adapter_test.rb | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 1c7c977141..a65771648e 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,3 +1,8 @@ +*Edge* + +* Added SQL escaping for :limit and :offset in MySQL [Jonathan Wiess] + + *2.1.0 (May 31st, 2008)* * Add ActiveRecord::Base.sti_name that checks ActiveRecord::Base#store_full_sti_class? and returns either the full or demodulized name. [rick] diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index f00a2c8950..653b45021d 100755 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -336,10 +336,11 @@ module ActiveRecord def add_limit_offset!(sql, options) #:nodoc: if limit = options[:limit] + limit = sanitize_limit(limit) unless offset = options[:offset] sql << " LIMIT #{limit}" else - sql << " LIMIT #{offset}, #{limit}" + sql << " LIMIT #{offset.to_i}, #{limit}" end end end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index c77446f880..11f9870534 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -118,7 +118,7 @@ class AdapterTest < ActiveRecord::TestCase sql_inject = "1, 7 procedure help()" if current_adapter?(:MysqlAdapter) assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) - assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) + assert_equal " LIMIT 7, 1", @connection.add_limit_offset!("", :limit=> '1 ; DROP TABLE USERS', :offset=>7) else assert_equal " LIMIT 1,7", @connection.add_limit_offset!("", :limit=>sql_inject) assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7) -- cgit v1.2.3 From 4210d85a3f3ce4e980f473e9ed2becb84b58363b Mon Sep 17 00:00:00 2001 From: Jonathan Viney Date: Mon, 2 Jun 2008 15:00:15 +1200 Subject: Ensure Associations#sum returns 0 when no rows are returned. [#295 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/calculations.rb | 6 +++--- activerecord/test/cases/calculations_test.rb | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 10e8330d1c..889b7845fb 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -71,7 +71,7 @@ module ActiveRecord # # Person.sum('age') def sum(column_name, options = {}) - calculate(:sum, column_name, options) || 0 + calculate(:sum, column_name, options) end # This calculates aggregate values in the given column. Methods for count, sum, average, minimum, and maximum have been added as shortcuts. @@ -265,8 +265,8 @@ module ActiveRecord def type_cast_calculated_value(value, column, operation = nil) operation = operation.to_s.downcase case operation - when 'count' then value.to_i - when 'avg' then value && value.to_f + when 'count', 'sum' then value.to_i + when 'avg' then value && value.to_f else column ? column.type_cast(value) : value end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index a87fc26905..5cf655f7b7 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -99,6 +99,7 @@ class CalculationsTest < ActiveRecord::TestCase def test_should_return_zero_if_sum_conditions_return_nothing assert_equal 0, Account.sum(:credit_limit, :conditions => '1 = 2') + assert_equal 0, companies(:rails_core).companies.sum(:id, :conditions => '1 = 2') end def test_should_group_by_summed_field_with_conditions @@ -266,6 +267,6 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_sum_expression - assert_equal "636", Account.sum("2 * credit_limit") + assert_equal 636, Account.sum("2 * credit_limit") end end -- cgit v1.2.3 From bd75a722a2e9f979bfd1b1d89442e4dd6f3e3af7 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Mon, 2 Jun 2008 20:40:25 +0100 Subject: Ensure AR#sum result is typecasted properly --- activerecord/lib/active_record/calculations.rb | 5 +++-- activerecord/test/cases/calculations_test.rb | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 889b7845fb..caa8c539d5 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -265,8 +265,9 @@ module ActiveRecord def type_cast_calculated_value(value, column, operation = nil) operation = operation.to_s.downcase case operation - when 'count', 'sum' then value.to_i - when 'avg' then value && value.to_f + when 'count' then value.to_i + when 'sum' then value =~ /\./ ? value.to_f : value.to_i + when 'avg' then value && value.to_f else column ? column.type_cast(value) : value end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 5cf655f7b7..aefb13ea9a 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -102,6 +102,11 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 0, companies(:rails_core).companies.sum(:id, :conditions => '1 = 2') end + def test_sum_should_return_valid_values_for_decimals + NumericData.create(:bank_balance => 19.83) + assert_equal 19.83, NumericData.sum(:bank_balance) + end + def test_should_group_by_summed_field_with_conditions c = Account.sum(:credit_limit, :conditions => 'firm_id > 1', :group => :firm_id) -- cgit v1.2.3 From c08547d2266c75f0a82d06dd91c6d0500740e12e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 3 Jun 2008 13:32:53 -0500 Subject: Namespace Inflector, Dependencies, OrderedOptions, and TimeZone under ActiveSupport [#238 state:resolved] --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 20 ++++++++-------- activerecord/test/cases/base_test.rb | 28 +++++++++++------------ activerecord/test/cases/inheritance_test.rb | 4 ++-- activerecord/test/cases/multiple_db_test.rb | 2 +- 5 files changed, 28 insertions(+), 28 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 92a24ecc5f..8dd07eb478 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -372,7 +372,7 @@ module ActiveRecord #:nodoc: def self.reset_subclasses #:nodoc: nonreloadables = [] subclasses.each do |klass| - unless Dependencies.autoloaded? klass + unless ActiveSupport::Dependencies.autoloaded? klass nonreloadables << klass next end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index c336fd9afb..7999e29264 100755 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -137,7 +137,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end end - + def test_time_attributes_are_retrieved_in_current_time_zone in_time_zone "Pacific Time (US & Canada)" do utc_time = Time.utc(2008, 1, 1) @@ -145,7 +145,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record[:written_on] = utc_time assert_equal utc_time, record.written_on # record.written on is equal to (i.e., simultaneous with) utc_time assert_kind_of ActiveSupport::TimeWithZone, record.written_on # but is a TimeWithZone - assert_equal TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone # and is in the current Time.zone + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone # and is in the current Time.zone assert_equal Time.utc(2007, 12, 31, 16), record.written_on.time # and represents time values adjusted accordingly end end @@ -156,7 +156,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = utc_time assert_equal utc_time, record.written_on - assert_equal TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone assert_equal Time.utc(2007, 12, 31, 16), record.written_on.time end end @@ -168,7 +168,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = cst_time assert_equal utc_time, record.written_on - assert_equal TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone assert_equal Time.utc(2007, 12, 31, 16), record.written_on.time end end @@ -181,12 +181,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = time_string assert_equal Time.zone.parse(time_string), record.written_on - assert_equal TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone assert_equal Time.utc(2007, 12, 31, 16), record.written_on.time end end end - + def test_setting_time_zone_aware_attribute_to_blank_string_returns_nil in_time_zone "Pacific Time (US & Canada)" do record = @target.new @@ -202,7 +202,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = time_string assert_equal Time.zone.parse(time_string), record.written_on - assert_equal TimeZone[timezone_offset], record.written_on.time_zone + assert_equal ActiveSupport::TimeZone[timezone_offset], record.written_on.time_zone assert_equal Time.utc(2008, 1, 1), record.written_on.time end end @@ -214,7 +214,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = utc_time.in_time_zone assert_equal utc_time, record.written_on - assert_equal TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone assert_equal Time.utc(2007, 12, 31, 16), record.written_on.time end end @@ -223,12 +223,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase def time_related_columns_on_topic Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name) end - + def in_time_zone(zone) old_zone = Time.zone old_tz = ActiveRecord::Base.time_zone_aware_attributes - Time.zone = zone ? TimeZone[zone] : nil + Time.zone = zone ? ActiveSupport::TimeZone[zone] : nil ActiveRecord::Base.time_zone_aware_attributes = !zone.nil? yield ensure diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e07ec50c46..a4be629fbd 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -133,19 +133,19 @@ class BasicsTest < ActiveRecord::TestCase category_attrs = {"name"=>"Test categoty", "type" => nil} assert_equal category_attrs , category.attributes_before_type_cast end - + if current_adapter?(:MysqlAdapter) def test_read_attributes_before_type_cast_on_boolean bool = Booleantest.create({ "value" => false }) assert_equal 0, bool.attributes_before_type_cast["value"] end end - + def test_read_attributes_before_type_cast_on_datetime developer = Developer.find(:first) assert_equal developer.created_at.to_s(:db) , developer.attributes_before_type_cast["created_at"] end - + def test_hash_content topic = Topic.new topic.content = { "one" => 1, "two" => 2 } @@ -251,7 +251,7 @@ class BasicsTest < ActiveRecord::TestCase topic = Topic.create("title" => "New Topic") topicReloaded = Topic.find(topic.id) assert_equal(topic, topicReloaded) - end + end def test_create_through_factory_with_block topic = Topic.create("title" => "New Topic") do |t| @@ -576,7 +576,7 @@ class BasicsTest < ActiveRecord::TestCase def test_destroy_all original_count = Topic.count topics_by_mary = Topic.count(:conditions => mary = "author_name = 'Mary'") - + Topic.destroy_all mary assert_equal original_count - topics_by_mary, Topic.count end @@ -665,7 +665,7 @@ class BasicsTest < ActiveRecord::TestCase def test_delete_all assert Topic.count > 0 - + assert_equal Topic.count, Topic.delete_all end @@ -970,7 +970,7 @@ class BasicsTest < ActiveRecord::TestCase topic.attributes = attributes assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end - + def test_multiparameter_attributes_on_time_with_old_date attributes = { "written_on(1i)" => "1850", "written_on(2i)" => "6", "written_on(3i)" => "24", @@ -998,7 +998,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes ActiveRecord::Base.time_zone_aware_attributes = true ActiveRecord::Base.default_timezone = :utc - Time.zone = TimeZone[-28800] + Time.zone = ActiveSupport::TimeZone[-28800] attributes = { "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" @@ -1016,7 +1016,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes_false ActiveRecord::Base.time_zone_aware_attributes = false - Time.zone = TimeZone[-28800] + Time.zone = ActiveSupport::TimeZone[-28800] attributes = { "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" @@ -1032,7 +1032,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_time_with_skip_time_zone_conversion_for_attributes ActiveRecord::Base.time_zone_aware_attributes = true ActiveRecord::Base.default_timezone = :utc - Time.zone = TimeZone[-28800] + Time.zone = ActiveSupport::TimeZone[-28800] Topic.skip_time_zone_conversion_for_attributes = [:written_on] attributes = { "written_on(1i)" => "2004", "written_on(2i)" => "6", "written_on(3i)" => "24", @@ -1647,7 +1647,7 @@ class BasicsTest < ActiveRecord::TestCase last = Developer.find :last assert_equal last, Developer.find(:first, :order => 'id desc') end - + def test_last assert_equal Developer.find(:first, :order => 'id desc'), Developer.last end @@ -1655,7 +1655,7 @@ class BasicsTest < ActiveRecord::TestCase def test_all_with_conditions assert_equal Developer.find(:all, :order => 'id desc'), Developer.all(:order => 'id desc') end - + def test_find_ordered_last last = Developer.find :last, :order => 'developers.salary ASC' assert_equal last, Developer.find(:all, :order => 'developers.salary ASC').last @@ -1670,14 +1670,14 @@ class BasicsTest < ActiveRecord::TestCase last = Developer.find :last, :order => 'developers.name, developers.salary DESC' assert_equal last, Developer.find(:all, :order => 'developers.name, developers.salary DESC').last end - + def test_find_scoped_ordered_last last_developer = Developer.with_scope(:find => { :order => 'developers.salary ASC' }) do Developer.find(:last) end assert_equal last_developer, Developer.find(:all, :order => 'developers.salary ASC').last end - + def test_abstract_class assert !ActiveRecord::Base.abstract_class? assert LoosePerson.abstract_class? diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index f09b617e6d..47fb5c608f 100755 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -223,11 +223,11 @@ class InheritanceComputeTypeTest < ActiveRecord::TestCase fixtures :companies def setup - Dependencies.log_activity = true + ActiveSupport::Dependencies.log_activity = true end def teardown - Dependencies.log_activity = false + ActiveSupport::Dependencies.log_activity = false self.class.const_remove :FirmOnTheFly rescue nil Firm.const_remove :FirmOnTheFly rescue nil end diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index 236e4710a4..eb3e43c8ac 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -51,7 +51,7 @@ class MultipleDbTest < ActiveRecord::TestCase def test_course_connection_should_survive_dependency_reload assert Course.connection - Dependencies.clear + ActiveSupport::Dependencies.clear Object.send(:remove_const, :Course) require_dependency 'models/course' -- cgit v1.2.3 From aa1771668877f20ca044e8f45a9736fbb7c8402e Mon Sep 17 00:00:00 2001 From: Craig Demyanovich Date: Tue, 3 Jun 2008 13:38:00 -0500 Subject: Callbacks fire before notifying observers [#230 state:resolved] Signed-off-by: Joshua Peek --- activerecord/lib/active_record/callbacks.rb | 4 +-- .../test/cases/callbacks_observers_test.rb | 38 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 activerecord/test/cases/callbacks_observers_test.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 41ec5c5e61..4edc209c65 100755 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -293,14 +293,14 @@ module ActiveRecord private def callback(method) - notify(method) - result = run_callbacks(method) { |result, object| result == false } if result != false && respond_to_without_attributes?(method) result = send(method) end + notify(method) + return result end diff --git a/activerecord/test/cases/callbacks_observers_test.rb b/activerecord/test/cases/callbacks_observers_test.rb new file mode 100644 index 0000000000..87de524923 --- /dev/null +++ b/activerecord/test/cases/callbacks_observers_test.rb @@ -0,0 +1,38 @@ +require "cases/helper" + +class Comment < ActiveRecord::Base + attr_accessor :callers + + before_validation :record_callers + + def after_validation + record_callers + end + + def record_callers + callers << self.class if callers + end +end + +class CommentObserver < ActiveRecord::Observer + attr_accessor :callers + + def after_validation(model) + callers << self.class if callers + end +end + +class CallbacksObserversTest < ActiveRecord::TestCase + def test_model_callbacks_fire_before_observers_are_notified + callers = [] + + comment = Comment.new + comment.callers = callers + + CommentObserver.instance.callers = callers + + comment.valid? + + assert_equal [Comment, Comment, CommentObserver], callers, "model callbacks did not fire before observers were notified" + end +end -- cgit v1.2.3