From 8a77c4abfa760e5829b566698400f2115409b7ff Mon Sep 17 00:00:00 2001 From: Ken Miller Date: Wed, 15 Oct 2008 17:34:57 -0700 Subject: Fixed issue where block is not called on the very first invocation of a find_or_create_by_ automatic finder. [#1224 state:committed] --- activerecord/lib/active_record/base.rb | 4 ++-- activerecord/test/cases/finder_test.rb | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 039e14435f..b89c8641aa 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1764,7 +1764,7 @@ module ActiveRecord #:nodoc: # # Each dynamic finder or initializer/creator is also defined in the class after it is first invoked, so that future # attempts to use it do not run through method_missing. - def method_missing(method_id, *arguments) + def method_missing(method_id, *arguments, &block) if match = DynamicFinderMatch.match(method_id) attribute_names = match.attribute_names super unless all_attributes_exists?(attribute_names) @@ -1819,7 +1819,7 @@ module ActiveRecord #:nodoc: end end }, __FILE__, __LINE__ - send(method_id, *arguments) + send(method_id, *arguments, &block) end else super diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 853474916c..153880afbd 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -846,6 +846,17 @@ class FinderTest < ActiveRecord::TestCase assert !c.new_record? end + def test_find_or_create_should_work_with_block_on_first_call + class << Company + undef_method(:find_or_create_by_name) if method_defined?(:find_or_create_by_name) + end + c = Company.find_or_create_by_name(:name => "Fortune 1000") { |f| f.rating = 1000 } + assert_equal "Fortune 1000", c.name + assert_equal 1000.to_f, c.rating.to_f + assert c.valid? + assert !c.new_record? + end + def test_dynamic_find_or_initialize_from_one_attribute_caches_method class << Company; self; end.send(:remove_method, :find_or_initialize_by_name) if Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } assert !Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } -- cgit v1.2.3 From 5c97d4ff29cfd944da751f01177a3024626d57bb Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Sat, 25 Oct 2008 14:49:39 +0530 Subject: "raise NoMethodError" raises NoMethodError. Raise it with NoMethodError.new instead. Signed-off-by: Pratik Naik --- activerecord/lib/active_record/attribute_methods.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 1c753524de..177d156834 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -233,7 +233,7 @@ module ActiveRecord method_name = method_id.to_s if self.class.private_method_defined?(method_name) - raise NoMethodError("Attempt to call private method", method_name, args) + raise NoMethodError.new("Attempt to call private method", method_name, args) end # If we haven't generated any methods yet, generate them, then diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 160716f944..77ee8d8fc4 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -233,8 +233,9 @@ class AttributeMethodsTest < ActiveRecord::TestCase topic = @target.new(:title => "The pros and cons of programming naked.") assert !topic.respond_to?(:title) - assert_raise(NoMethodError) { topic.title } - topic.send(:title) + exception = assert_raise(NoMethodError) { topic.title } + assert_equal "Attempt to call private method", exception.message + assert_equal "I'm private", topic.send(:title) end def test_write_attributes_respect_access_control @@ -242,7 +243,8 @@ class AttributeMethodsTest < ActiveRecord::TestCase topic = @target.new assert !topic.respond_to?(:title=) - assert_raise(NoMethodError) { topic.title = "Pants"} + exception = assert_raise(NoMethodError) { topic.title = "Pants"} + assert_equal "Attempt to call private method", exception.message topic.send(:title=, "Very large pants") end @@ -251,7 +253,8 @@ class AttributeMethodsTest < ActiveRecord::TestCase topic = @target.new(:title => "Isaac Newton's pants") assert !topic.respond_to?(:title?) - assert_raise(NoMethodError) { topic.title? } + exception = assert_raise(NoMethodError) { topic.title? } + assert_equal "Attempt to call private method", exception.message assert topic.send(:title?) end -- cgit v1.2.3 From 932dffc559ef188eb31d0223116e9da361833488 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Fri, 19 Sep 2008 21:38:39 -0500 Subject: Fix binary data corruption bug in PostgreSQL adaptor 1. Move the binary escape/unescape from column to the driver - we should store binary data AR just like most other adaptors 2. check to make sure we only unescape bytea data PGresult.ftype( column ) == 17 that is passed to us in escaped format PGresult.fformat( column ) == 0 Signed-off-by: Michael Koziarski [#1063 state:committed] --- .../connection_adapters/postgresql_adapter.rb | 147 +++++++++++---------- 1 file changed, 79 insertions(+), 68 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index bebab5d05d..ccde8a63a9 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -68,72 +68,6 @@ module ActiveRecord super end - # Escapes binary strings for bytea input to the database. - def self.string_to_binary(value) - if PGconn.respond_to?(:escape_bytea) - self.class.module_eval do - define_method(:string_to_binary) do |value| - PGconn.escape_bytea(value) if value - end - end - else - self.class.module_eval do - define_method(:string_to_binary) do |value| - if value - result = '' - value.each_byte { |c| result << sprintf('\\\\%03o', c) } - result - end - end - end - end - self.class.string_to_binary(value) - end - - # Unescapes bytea output from a database to the binary string it represents. - def self.binary_to_string(value) - # In each case, check if the value actually is escaped PostgreSQL bytea output - # or an unescaped Active Record attribute that was just written. - if PGconn.respond_to?(:unescape_bytea) - self.class.module_eval do - define_method(:binary_to_string) do |value| - if value =~ /\\\d{3}/ - PGconn.unescape_bytea(value) - else - value - end - end - end - else - self.class.module_eval do - define_method(:binary_to_string) do |value| - if value =~ /\\\d{3}/ - result = '' - i, max = 0, value.size - while i < max - char = value[i] - if char == ?\\ - if value[i+1] == ?\\ - char = ?\\ - i += 1 - else - char = value[i+1..i+3].oct - i += 3 - end - end - result << char - i += 1 - end - result - else - value - end - end - end - end - self.class.binary_to_string(value) - end - # Maps PostgreSQL-specific data types to logical Rails types. def simplified_type(field_type) case field_type @@ -347,10 +281,78 @@ module ActiveRecord # QUOTING ================================================== + # Escapes binary strings for bytea input to the database. + def escape_bytea(value) + if PGconn.respond_to?(:escape_bytea) + self.class.instance_eval do + define_method(:escape_bytea) do |value| + PGconn.escape_bytea(value) if value + end + end + else + self.class.instance_eval do + define_method(:escape_bytea) do |value| + if value + result = '' + value.each_byte { |c| result << sprintf('\\\\%03o', c) } + result + end + end + end + end + escape_bytea(value) + end + + # Unescapes bytea output from a database to the binary string it represents. + # NOTE: This is NOT an inverse of escape_bytea! This is only to be used + # on escaped binary output from database drive. + def unescape_bytea(value) + # In each case, check if the value actually is escaped PostgreSQL bytea output + # or an unescaped Active Record attribute that was just written. + if PGconn.respond_to?(:unescape_bytea) + self.class.instance_eval do + define_method(:unescape_bytea) do |value| + if value =~ /\\\d{3}/ + PGconn.unescape_bytea(value) + else + value + end + end + end + else + self.class.instance_eval do + define_method(:unescape_bytea) do |value| + if value =~ /\\\d{3}/ + result = '' + i, max = 0, value.size + while i < max + char = value[i] + if char == ?\\ + if value[i+1] == ?\\ + char = ?\\ + i += 1 + else + char = value[i+1..i+3].oct + i += 3 + end + end + result << char + i += 1 + end + result + else + value + end + end + end + end + unescape_bytea(value) + end + # Quotes PostgreSQL-specific data types for SQL input. def quote(value, column = nil) #:nodoc: if value.kind_of?(String) && column && column.type == :binary - "#{quoted_string_prefix}'#{column.class.string_to_binary(value)}'" + "#{quoted_string_prefix}'#{escape_bytea(value)}'" elsif value.kind_of?(String) && column && column.sql_type =~ /^xml$/ "xml '#{quote_string(value)}'" elsif value.kind_of?(Numeric) && column && column.sql_type =~ /^money$/ @@ -463,11 +465,20 @@ module ActiveRecord # create a 2D array representing the result set def result_as_array(res) #:nodoc: + # check if we have any binary column and if they need escaping + unescape_col = [] + for j in 0...res.nfields do + # unescape string passed BYTEA field (OID == 17) + unescape_col << ( res.fformat(j)==0 and res.ftype(j)==17 ) + end + ary = [] for i in 0...res.ntuples do ary << [] for j in 0...res.nfields do - ary[i] << res.getvalue(i,j) + data = res.getvalue(i,j) + data = unescape_bytea(data) if unescape_col[j] and data.is_a?(String) + ary[i] << data end end return ary -- cgit v1.2.3 From 9e2bb2caff2b6fd4712ca3db258b68a588a69e9a Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Sun, 26 Oct 2008 20:04:09 +0100 Subject: Remove reference to fformat to restore support for postgres gem. --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index ccde8a63a9..60ec01b95e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -469,7 +469,7 @@ module ActiveRecord unescape_col = [] for j in 0...res.nfields do # unescape string passed BYTEA field (OID == 17) - unescape_col << ( res.fformat(j)==0 and res.ftype(j)==17 ) + unescape_col << ( res.ftype(j)==17 ) end ary = [] -- cgit v1.2.3 From c94ba8150a726da4a894cd8325ee682a3286ec9f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 27 Oct 2008 17:16:45 +0100 Subject: Fixed that serialized strings should never be type-casted (i.e. turning "Yes" to a boolean)(Andreas Korth) [#857 state:committed] --- activerecord/CHANGELOG | 5 +++++ activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/base_test.rb | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index fec110d569..06a290931c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,3 +1,8 @@ +*2.2.1 [RC2 or 2.2 final]* + +* Fixed that serialized strings should never be type-casted (i.e. turning "Yes" to a boolean) #857 [Andreas Korth] + + *2.2.0 [RC1] (October 24th, 2008)* * Skip collection ids reader optimization if using :finder_sql [Jeremy Kemper] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b89c8641aa..9c5690d3fd 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2938,7 +2938,7 @@ module ActiveRecord #:nodoc: end def object_from_yaml(string) - return string unless string.is_a?(String) + return string unless string.is_a?(String) && string =~ /^---/ YAML::load(string) rescue string end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d512834237..da9f2742d8 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1429,6 +1429,12 @@ class BasicsTest < ActiveRecord::TestCase topic = Topic.create("content" => myobj).reload assert_equal(myobj, topic.content) end + + def test_serialized_string_attribute + myobj = "Yes" + topic = Topic.create("content" => myobj).reload + assert_equal(myobj, topic.content) + end def test_nil_serialized_attribute_with_class_constraint myobj = MyObject.new('value1', 'value2') -- cgit v1.2.3 From 8f0f07863727edaad1e59df4ab4ec159016917e7 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 29 Oct 2008 10:53:33 +0100 Subject: Make #destroy write 1 line into log (instead of 3) (Dmitry Sokurenko) [#689 status:committed] --- activerecord/lib/active_record/base.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9c5690d3fd..a36a137f0d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2410,10 +2410,11 @@ module ActiveRecord #:nodoc: # be made (since they can't be persisted). def destroy unless new_record? - connection.delete <<-end_sql, "#{self.class.name} Destroy" - DELETE FROM #{self.class.quoted_table_name} - WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quoted_id} - end_sql + connection.delete( + "DELETE FROM #{self.class.quoted_table_name} " + + "WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quoted_id}", + "#{self.class.name} Destroy" + ) end freeze -- cgit v1.2.3 From a9e816843289d8839d4206c5ad09e49452d076ac Mon Sep 17 00:00:00 2001 From: Jordi Bunster Date: Fri, 24 Oct 2008 12:49:06 -0400 Subject: Ensure indices don't flip order in schema.rb [#1266 state:committed] Signed-off-by: David Heinemeier Hansson --- activerecord/CHANGELOG | 2 ++ activerecord/lib/active_record/schema_dumper.rb | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 06a290931c..4ca062b535 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *2.2.1 [RC2 or 2.2 final]* +* Ensure indices don't flip order in schema.rb #1266 [Jordi Bunster] + * Fixed that serialized strings should never be type-casted (i.e. turning "Yes" to a boolean) #857 [Andreas Korth] diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 4f96e225c1..2181bdf2dd 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -159,13 +159,19 @@ HEADER end def indexes(table, stream) - indexes = @connection.indexes(table) - indexes.each do |index| - stream.print " add_index #{index.table.inspect}, #{index.columns.inspect}, :name => #{index.name.inspect}" - stream.print ", :unique => true" if index.unique + if (indexes = @connection.indexes(table)).any? + add_index_statements = indexes.map do |index| + statment_parts = [ ('add_index ' + index.table.inspect) ] + statment_parts << index.columns.inspect + statment_parts << (':name => ' + index.name.inspect) + statment_parts << ':unique => true' if index.unique + + ' ' + statment_parts.join(', ') + end + + stream.puts add_index_statements.sort.join("\n") stream.puts end - stream.puts unless indexes.empty? end end -end +end \ No newline at end of file -- cgit v1.2.3 From 0c84b6f9eda20c30b66d8fb99fba637edc1bc37a Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 30 Oct 2008 15:48:03 -0500 Subject: Use database name in query cache thread local key [#1283 state:resolved] --- .../lib/active_record/connection_adapters/abstract/query_cache.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 2fc50b9bfa..8543bbf872 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -34,17 +34,16 @@ module ActiveRecord end def query_cache - Thread.current['query_cache'] + Thread.current["query_cache_for_#{@config[:database]}"] ||= {} end def query_cache=(cache) - Thread.current['query_cache'] = cache + Thread.current["query_cache_for_#{@config[:database]}"] = cache end # Enable the query cache within the block. def cache old, self.query_cache_enabled = query_cache_enabled, true - self.query_cache ||= {} yield ensure clear_query_cache -- cgit v1.2.3 From 4df5e1ae8fe3f5f2cf90c91ae4fb6324daf80082 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 30 Oct 2008 16:27:13 -0500 Subject: It is not necessary to store QueryCache in a thread local since the cache is local to the connection object which is managed by the connection pool --- .../connection_adapters/abstract/query_cache.rb | 41 ++++++++-------------- 1 file changed, 14 insertions(+), 27 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 8543bbf872..950bd72101 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -15,7 +15,7 @@ module ActiveRecord method_names.each do |method_name| base.class_eval <<-end_code, __FILE__, __LINE__ def #{method_name}_with_query_dirty(*args) - clear_query_cache if query_cache_enabled + clear_query_cache if @query_cache_enabled #{method_name}_without_query_dirty(*args) end @@ -25,37 +25,24 @@ module ActiveRecord end end - def query_cache_enabled - Thread.current['query_cache_enabled'] - end - - def query_cache_enabled=(flag) - Thread.current['query_cache_enabled'] = flag - end - - def query_cache - Thread.current["query_cache_for_#{@config[:database]}"] ||= {} - end - - def query_cache=(cache) - Thread.current["query_cache_for_#{@config[:database]}"] = cache - end + attr_reader :query_cache, :query_cache_enabled # Enable the query cache within the block. def cache - old, self.query_cache_enabled = query_cache_enabled, true + old, @query_cache_enabled = @query_cache_enabled, true + @query_cache ||= {} yield ensure clear_query_cache - self.query_cache_enabled = old + @query_cache_enabled = old end # Disable the query cache within the block. def uncached - old, self.query_cache_enabled = query_cache_enabled, false + old, @query_cache_enabled = @query_cache_enabled, false yield ensure - self.query_cache_enabled = old + @query_cache_enabled = old end # Clears the query cache. @@ -65,11 +52,11 @@ module ActiveRecord # the same SQL query and repeatedly return the same result each time, silently # undermining the randomness you were expecting. def clear_query_cache - query_cache.clear if query_cache + @query_cache.clear if @query_cache end def select_all_with_query_cache(*args) - if query_cache_enabled + if @query_cache_enabled cache_sql(args.first) { select_all_without_query_cache(*args) } else select_all_without_query_cache(*args) @@ -77,8 +64,8 @@ module ActiveRecord end def columns_with_query_cache(*args) - if query_cache_enabled - query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args) + if @query_cache_enabled + @query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args) else columns_without_query_cache(*args) end @@ -87,11 +74,11 @@ module ActiveRecord private def cache_sql(sql) result = - if query_cache.has_key?(sql) + if @query_cache.has_key?(sql) log_info(sql, "CACHE", 0.0) - query_cache[sql] + @query_cache[sql] else - query_cache[sql] = yield + @query_cache[sql] = yield end if Array === result -- cgit v1.2.3 From 5229fc4cc034ac5f565d143f5fd59ac11ebdc8e3 Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Fri, 31 Oct 2008 17:28:16 +0100 Subject: Make sure habtm use class variable to list association valid keys Signed-off-by: Michael Koziarski [#1310 state:committed] --- activerecord/lib/active_record/associations.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 52f6a04da1..84caca3518 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1596,16 +1596,19 @@ module ActiveRecord reflection end + mattr_accessor :valid_keys_for_has_and_belongs_to_many_association + @@valid_keys_for_has_and_belongs_to_many_association = [ + :class_name, :table_name, :join_table, :foreign_key, :association_foreign_key, + :select, :conditions, :include, :order, :group, :limit, :offset, + :uniq, + :finder_sql, :delete_sql, :insert_sql, + :before_add, :after_add, :before_remove, :after_remove, + :extend, :readonly, + :validate + ] + def create_has_and_belongs_to_many_reflection(association_id, options, &extension) - options.assert_valid_keys( - :class_name, :table_name, :join_table, :foreign_key, :association_foreign_key, - :select, :conditions, :include, :order, :group, :limit, :offset, - :uniq, - :finder_sql, :delete_sql, :insert_sql, - :before_add, :after_add, :before_remove, :after_remove, - :extend, :readonly, - :validate - ) + options.assert_valid_keys(valid_keys_for_has_and_belongs_to_many_association) options[:extend] = create_extension_modules(association_id, extension, options[:extend]) -- cgit v1.2.3