From e75314803e1548aa3b25657373003cde4567df64 Mon Sep 17 00:00:00 2001 From: Akshay Vishnoi Date: Sun, 13 Apr 2014 21:20:31 +0530 Subject: [ci skip] Improve doc, fix grammatical issue --- activerecord/lib/active_record/nested_attributes.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 9d92e747d4..e6195e48a5 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -485,10 +485,10 @@ module ActiveRecord end # Takes in a limit and checks if the attributes_collection has too many - # records. The method will take limits in the form of symbols, procs, and - # number-like objects (anything that can be compared with an integer). + # records. It accepts limit in the form of symbol, proc, or + # number-like object (anything that can be compared with an integer). # - # Will raise an TooManyRecords error if the attributes_collection is + # Raises TooManyRecords error if the attributes_collection is # larger than the limit. def check_record_limit!(limit, attributes_collection) if limit @@ -519,7 +519,7 @@ module ActiveRecord ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) end - # Determines if a new record should be build by checking for + # Determines if a new record should be rejected by checking # has_destroy_flag? or if a :reject_if proc exists for this # association and evaluates to +true+. def reject_new_record?(association_name, attributes) -- cgit v1.2.3 From 109d1b2b10ac8b602b6a1a9995c3fd9c63aefa22 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 14 Apr 2014 14:28:26 -0400 Subject: Use inheritance chain instead of callbacks to increment counter caches after create --- .../associations/belongs_to_association.rb | 18 ++++++++++++++---- .../active_record/associations/builder/belongs_to.rb | 13 +------------ activerecord/lib/active_record/counter_cache.rb | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 16 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 8272a5584c..b0820f662a 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -31,6 +31,14 @@ module ActiveRecord @updated end + def decrement_counters + with_cache_name { |name| decrement_counter name } + end + + def increment_counters + with_cache_name { |name| increment_counter name } + end + private def find_target? @@ -51,13 +59,15 @@ module ActiveRecord end end - def decrement_counters - with_cache_name { |name| decrement_counter name } + def decrement_counter(counter_cache_name) + if foreign_key_present? + klass.decrement_counter(counter_cache_name, target_id) + end end - def decrement_counter counter_cache_name + def increment_counter(counter_cache_name) if foreign_key_present? - klass.decrement_counter(counter_cache_name, target_id) + klass.increment_counter(counter_cache_name, target_id) end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 11be92ae01..bd2ee1a929 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -26,16 +26,9 @@ module ActiveRecord::Associations::Builder private def self.add_counter_cache_methods(mixin) - return if mixin.method_defined? :belongs_to_counter_cache_after_create + return if mixin.method_defined? :belongs_to_counter_cache_after_update mixin.class_eval do - def belongs_to_counter_cache_after_create(reflection) - if record = send(reflection.name) - cache_column = reflection.counter_cache_column - record.class.increment_counter(cache_column, record.id) - @_after_create_counter_called = true - end - end def belongs_to_counter_cache_after_destroy(reflection) foreign_key = reflection.foreign_key.to_sym @@ -74,10 +67,6 @@ module ActiveRecord::Associations::Builder def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - model.after_create lambda { |record| - record.belongs_to_counter_cache_after_create(reflection) - } - model.after_destroy lambda { |record| record.belongs_to_counter_cache_after_destroy(reflection) } diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index a5897edf03..163da8e870 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -131,6 +131,16 @@ module ActiveRecord private + def _create_record(*) + id = super + + each_counter_cached_associations do |association| + association.increment_counters + end + + id + end + def destroy_row affected_rows = super @@ -139,5 +149,11 @@ module ActiveRecord affected_rows end + def each_counter_cached_associations + reflections.each do |name, reflection| + yield association(name) if reflection.belongs_to? && reflection.counter_cache_column + end + end + end end -- cgit v1.2.3 From 7e28b4ed9a1dbd131877cf784aad7ace49073a52 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 14 Apr 2014 14:55:34 -0400 Subject: Use inheritance chain instead of callbacks to increment counter caches after destroy --- .../lib/active_record/associations/builder/belongs_to.rb | 16 ---------------- activerecord/lib/active_record/counter_cache.rb | 8 ++++---- 2 files changed, 4 insertions(+), 20 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index bd2ee1a929..7614fdd97f 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -30,18 +30,6 @@ module ActiveRecord::Associations::Builder mixin.class_eval do - def belongs_to_counter_cache_after_destroy(reflection) - foreign_key = reflection.foreign_key.to_sym - unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key - record = send reflection.name - if record && self.actually_destroyed? - cache_column = reflection.counter_cache_column - record.class.decrement_counter(cache_column, record.id) - self.clear_destroy_state - end - end - end - def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column @@ -67,10 +55,6 @@ module ActiveRecord::Associations::Builder def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - model.after_destroy lambda { |record| - record.belongs_to_counter_cache_after_destroy(reflection) - } - model.after_update lambda { |record| record.belongs_to_counter_cache_after_update(reflection) } diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 163da8e870..1872d0c141 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -134,9 +134,7 @@ module ActiveRecord def _create_record(*) id = super - each_counter_cached_associations do |association| - association.increment_counters - end + each_counter_cached_associations(&:increment_counters) id end @@ -144,7 +142,9 @@ module ActiveRecord def destroy_row affected_rows = super - @_actually_destroyed = affected_rows > 0 + if affected_rows > 0 + each_counter_cached_associations(&:decrement_counters) + end affected_rows end -- cgit v1.2.3 From ed6894bf2386cf469018a5b32e60568c8df20e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 14 Apr 2014 14:40:38 -0300 Subject: Remove outdated comment --- activerecord/test/models/pirate.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 7bb0caf44b..e472cf951d 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -18,7 +18,6 @@ class Pirate < ActiveRecord::Base has_many :treasures, :as => :looter has_many :treasure_estimates, :through => :treasures, :source => :price_estimates - # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship has_one :update_only_ship, :class_name => 'Ship' has_one :non_validated_ship, :class_name => 'Ship' -- cgit v1.2.3 From 34945e41c24c59268bbde63abfafe20bdc09b775 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Mon, 14 Apr 2014 21:24:31 -0300 Subject: The Association Relation should use `empty?` and `size` from Relation. 968c581ea34b5236af14805e6a77913b1cb36238 have introduced this bug #14744 on Association Relation when the method `empty?` or `size` was called. Example: # Given an author that does have 3 posts, but none of them with the # title 'Some Title' Author.last.posts.where(title: 'Some Title').size # => 3 It was occurring, because the Association Relation had implemented these methods based on `@association`, this way giving wrong results. To fix it, was necessary to remove the methods `empty?` and `size` from Association Relation. It just have to use these methods from Relation. Example: # Given an author that does have 3 posts, but none of them with the # title 'Some Title' Author.last.posts.where(title: 'Some Title').size # => 0 # Now it will return the correct value. Fixes #14744. --- activerecord/lib/active_record/association_relation.rb | 8 -------- activerecord/test/cases/relations_test.rb | 11 +++++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index 45f1b07f69..5a84792f45 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -9,14 +9,6 @@ module ActiveRecord @association end - def size - @association.size - end - - def empty? - @association.empty? - end - def ==(other) other == to_a end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 562cfe6796..ddfcaaf986 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -864,6 +864,17 @@ class RelationTest < ActiveRecord::TestCase assert_equal 9, posts.where(:comments_count => 0).count end + def test_count_on_association_relation + author = Author.last + another_author = Author.first + posts = Post.where(author_id: author.id) + + assert_equal author.posts.where(author_id: author.id).size, posts.count + + assert_equal 0, author.posts.where(author_id: another_author.id).size + assert author.posts.where(author_id: another_author.id).empty? + end + def test_count_with_distinct posts = Post.all -- cgit v1.2.3 From a9416a407b3e5256a0a3fbe6c6ee5112ca3b6703 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Mon, 14 Apr 2014 21:35:49 -0300 Subject: Make sure the column_name is different from 'all'. 968c581ea34b5236af14805e6a77913b1cb36238 have fixed the EagerLoadTest, but not in the correct way. The problem was when `empty?` or `size` was called on relation. It was triggering `count(:all)`, which was passing `:all` as the column name to `count` on Calculations. On the other hand, the method `calculate` on Calculations was calling `construct_relation_for_association_calculations` instead of `perform_calculation`, because `has_include?` was returning `true` since `column_name` was present. To prevent calling the wrong method to perform the calculation, we have to check if the `column_name` is present and if it is different from `:all` (which is now used to correctly do `count` with `select`). More information here: https://github.com/rails/rails/commit/968c581ea34b5236af14805e6a77913b1cb36238#commitcomment-6006135 --- activerecord/lib/active_record/relation/calculations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 812e3e800a..514ebc2bfe 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -188,7 +188,7 @@ module ActiveRecord private def has_include?(column_name) - eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) + eager_loading? || (includes_values.present? && ((column_name && column_name != :all) || references_eager_loaded_tables?)) end def perform_calculation(operation, column_name, options = {}) -- cgit v1.2.3 From b335def167f1141995810d32d700ef3f4d036cd4 Mon Sep 17 00:00:00 2001 From: Robin Tweedie Date: Tue, 15 Apr 2014 13:12:49 +0100 Subject: use YAML.load_file in database tasks example rather than YAML.load(File.read(path)). YAML.load_file is also used in guides/rails_guides/helper.rb since 2011, the only other precedent I could find. --- activerecord/lib/active_record/tasks/database_tasks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 6ce0495f6f..168b338b97 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -28,7 +28,7 @@ module ActiveRecord # Example usage of +DatabaseTasks+ outside Rails could look as such: # # include ActiveRecord::Tasks - # DatabaseTasks.database_configuration = YAML.load(File.read('my_database_config.yml')) + # DatabaseTasks.database_configuration = YAML.load_file('my_database_config.yml') # DatabaseTasks.db_dir = 'db' # # other settings... # -- cgit v1.2.3 From a5a254268b9c06dd58d6ea70a7729fdfb2ca14b3 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 15 Apr 2014 11:01:04 -0500 Subject: [ci skip] Use valid current config in example `reaping_frequency` is used in Active Record `reap_frequency` is not --- .../lib/active_record/connection_adapters/connection_specification.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 9a133168f8..41af636ef6 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -57,8 +57,8 @@ module ActiveRecord # Converts the query parameters of the URI into a hash. # - # "localhost?pool=5&reap_frequency=2" - # # => { "pool" => "5", "reap_frequency" => "2" } + # "localhost?pool=5&reaping_frequency=2" + # # => { "pool" => "5", "reaping_frequency" => "2" } # # returns empty hash if no query present. # -- cgit v1.2.3 From 5e32f976928e30da6d2017b415657950adf0c2a8 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 13:35:02 -0400 Subject: Set _after_create_counter_called flag to make update counter cache work --- activerecord/lib/active_record/counter_cache.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 1872d0c141..4108b46439 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -134,7 +134,12 @@ module ActiveRecord def _create_record(*) id = super - each_counter_cached_associations(&:increment_counters) + each_counter_cached_associations do |association| + if record = send(association.reflection.name) + association.increment_counters + @_after_create_counter_called = true + end + end id end -- cgit v1.2.3 From d2543412ff42d64c5fc5d763336b3f3ec9ab8eda Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 14:11:47 -0400 Subject: Restore the destroy_by_association check in post destroy counter cache --- activerecord/lib/active_record/counter_cache.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 4108b46439..b7b790322a 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -135,7 +135,7 @@ module ActiveRecord id = super each_counter_cached_associations do |association| - if record = send(association.reflection.name) + if send(association.reflection.name) association.increment_counters @_after_create_counter_called = true end @@ -148,7 +148,14 @@ module ActiveRecord affected_rows = super if affected_rows > 0 - each_counter_cached_associations(&:decrement_counters) + each_counter_cached_associations do |association| + foreign_key = association.reflection.foreign_key.to_sym + unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key + if send(association.reflection.name) + association.decrement_counters + end + end + end end affected_rows -- cgit v1.2.3 From 7af987cf297efcb0a03a168ff9486c43e0b2ff97 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 15 Apr 2014 14:21:35 -0400 Subject: Hide BelongsToAssociation#increment_counters and #decrement_counters --- activerecord/lib/active_record/associations/belongs_to_association.rb | 4 ++-- activerecord/lib/active_record/associations/builder/belongs_to.rb | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index b0820f662a..1edd4fa3aa 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -31,11 +31,11 @@ module ActiveRecord @updated end - def decrement_counters + def decrement_counters # :nodoc: with_cache_name { |name| decrement_counter name } end - def increment_counters + def increment_counters # :nodoc: with_cache_name { |name| increment_counter name } end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 7614fdd97f..47cc1f4b34 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -29,7 +29,6 @@ module ActiveRecord::Associations::Builder return if mixin.method_defined? :belongs_to_counter_cache_after_update mixin.class_eval do - def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column -- cgit v1.2.3 From eaa3949d36df60e6bbeccf187aa60547d6b9085b Mon Sep 17 00:00:00 2001 From: Eric Chahin Date: Tue, 15 Apr 2014 16:08:04 -0400 Subject: Changed change_column in PG schema_statements.rb to make sure that the uuid_generate function was not being quoted. --- .../active_record/connection_adapters/postgresql/quoting.rb | 9 +++++++++ .../connection_adapters/postgresql/schema_statements.rb | 10 ++++++++-- activerecord/test/cases/adapters/postgresql/uuid_test.rb | 13 +++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index ac3b0f713d..403e37fde9 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -182,6 +182,15 @@ module ActiveRecord end result end + + # Does not quote function default values for UUID columns + def quote_default_value(value, column) #:nodoc: + if column.type == :uuid && value =~ /\(\)/ + value + else + quote(value) + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 1229b4851a..1dc7a6f0fd 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -187,6 +187,10 @@ module ActiveRecord end end + def column_for(table_name, column_name) #:nodoc: + columns(table_name).detect { |c| c.name == column_name.to_s } + end + # Returns the current database name. def current_database query('select current_database()', 'SCHEMA')[0][0] @@ -404,13 +408,15 @@ module ActiveRecord # Changes the default value of a table column. def change_column_default(table_name, column_name, default) clear_cache! - execute "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} SET DEFAULT #{quote(default)}" + column = column_for(table_name, column_name) + execute "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} SET DEFAULT #{quote_default_value(default, column)}" if column end def change_column_null(table_name, column_name, null, default = nil) clear_cache! unless null || default.nil? - execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL") + column = column_for(table_name, column_name) + execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_value(default, column)} WHERE #{quote_column_name(column_name)} IS NULL") if column end execute("ALTER TABLE #{quote_table_name(table_name)} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL") end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 9e03ea6bee..bdf8e15e3e 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -40,6 +40,19 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase drop_table "uuid_data_type" end + def test_change_column_default + @connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()" + UUIDType.reset_column_information + column = UUIDType.columns.find { |c| c.name == 'thingy' } + assert_equal "uuid_generate_v1()", column.default_function + + @connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()" + + UUIDType.reset_column_information + column = UUIDType.columns.find { |c| c.name == 'thingy' } + assert_equal "uuid_generate_v4()", column.default_function + end + def test_data_type_of_uuid_types column = UUIDType.columns_hash["guid"] assert_equal :uuid, column.type -- cgit v1.2.3 From f02393cfd83e00348963b0e5d08ec40db9e4babf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 15 Apr 2014 17:24:49 -0300 Subject: Add CHANGELOG entry for #14766 [ci skip] --- activerecord/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a45d912017..62fa39b4f4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Do not quote uuid default value on `change_column`. + + Fixes #14604. + + *Eric Chahin* + * The comparison between `Relation` and `CollectionProxy` should be consistent. Example: -- cgit v1.2.3 From fe4b0eee05f59831e1468ed50f55fbad0ce11e1d Mon Sep 17 00:00:00 2001 From: Rob Gilson Date: Thu, 27 Feb 2014 13:34:21 -0500 Subject: SQL Like escaping helper method. [Rob Gilson & Yves Senn] Closes #14222. This is a follow up to #6104 This does not have the backwards compatibility issues brought up in implementation to break. --- activerecord/CHANGELOG.md | 16 ++++++++++++++++ activerecord/lib/active_record/sanitization.rb | 6 ++++++ activerecord/test/cases/sanitize_test.rb | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 62fa39b4f4..0a12049e5d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* `sanitize_sql_like` helper method to escape a string for safe use in a SQL + LIKE statement. + + Example: + + class Article + def self.search(term) + where("title LIKE ?", sanitize_sql_like(term)) + end + end + + Article.search("20% _reduction_") + # => Query looks like "... title LIKE '20\% \_reduction\_' ..." + + *Rob Gilson*, *Yves Senn* + * Do not quote uuid default value on `change_column`. Fixes #14604. diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 5a71c13d91..ef63949208 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -107,6 +107,12 @@ module ActiveRecord end.join(', ') end + # Sanitizes a +string+ so that it is safe to use within a sql + # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%" + def sanitize_sql_like(string, escape_character = "\\") + string.gsub(/[\\_%]/) { |x| [escape_character, x].join } + end + # Accepts an array of conditions. The array has each value # sanitized and interpolated into the SQL statement. # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 954eab8022..18182efc46 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -51,4 +51,30 @@ class SanitizeTest < ActiveRecord::TestCase select_author_sql = Post.send(:sanitize_sql_array, ['']) assert_equal('', select_author_sql) end + + def test_sanitize_sql_like + assert_equal '100\%', Binary.send(:sanitize_sql_like, '100%') + assert_equal 'snake\_cased\_string', Binary.send(:sanitize_sql_like, 'snake_cased_string') + assert_equal 'C:\\\\Programs\\\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint') + assert_equal 'normal string 42', Binary.send(:sanitize_sql_like, 'normal string 42') + end + + def test_sanitize_sql_like_with_custom_escape_character + assert_equal '100!%', Binary.send(:sanitize_sql_like, '100%', '!') + assert_equal 'snake!_cased!_string', Binary.send(:sanitize_sql_like, 'snake_cased_string', '!') + assert_equal 'C:!\\Programs!\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint', '!') + assert_equal 'normal string 42', Binary.send(:sanitize_sql_like, 'normal string 42', '!') + end + + def test_sanitize_sql_like_example_use_case + searchable_post = Class.new(Post) do + def self.search(term) + where("title LIKE ?", sanitize_sql_like(term)) + end + end + + assert_sql /LIKE '20\\% \\_reduction\\_'/ do + searchable_post.search("20% _reduction_").to_a + end + end end -- cgit v1.2.3 From 93f852569efc4db62e2fed75b2c0bb1866f7065b Mon Sep 17 00:00:00 2001 From: Eric Chahin Date: Wed, 16 Apr 2014 02:27:26 -0400 Subject: Changed the NullRelation so that when count is called with #group it will properly return an empty hash instead of zero. Fixes issue #14721 Conflicts: activerecord/CHANGELOG.md --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/null_relation.rb | 4 ++-- activerecord/test/cases/relations_test.rb | 11 +++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0a12049e5d..57a083c1df 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fixed a problem where count used with a grouping was not returning a Hash. + + Fixes #14721. + + *Eric Chahin* + * `sanitize_sql_like` helper method to escape a string for safe use in a SQL LIKE statement. diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 5b255c3fe5..05d0c41678 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -43,7 +43,7 @@ module ActiveRecord end def count(*) - 0 + calculate :count, nil end def sum(*) @@ -54,7 +54,7 @@ module ActiveRecord # TODO: Remove _options argument as soon we remove support to # activerecord-deprecated_finders. if operation == :count - 0 + group_values.any? ? Hash.new : 0 else nil end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index ddfcaaf986..d054dfa25a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -14,6 +14,7 @@ require 'models/car' require 'models/engine' require 'models/tyre' require 'models/minivan' +require 'models/aircraft' class RelationTest < ActiveRecord::TestCase @@ -365,6 +366,16 @@ class RelationTest < ActiveRecord::TestCase assert_equal({ 'salary' => 100_000 }, Developer.none.where(salary: 100_000).where_values_hash) end + + def test_null_relation_count + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:id).count + assert_equal 0, ac.engines.count + ac.save + assert_equal Hash.new, ac.engines.group(:id).count + assert_equal 0, ac.engines.count + end + def test_joins_with_nil_argument assert_nothing_raised { DependentFirm.joins(nil).first } end -- cgit v1.2.3 From 973a45230ab5ba0e096585ecd1403a13569a1348 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 16 Apr 2014 16:45:10 +0200 Subject: `sanitize_sql_like` escapes `escape_character` not only backslash. * This is a follow up to: fe4b0eee05f59831e1468ed50f55fbad0ce11e1d * The originating PR is #14222 * It should fix the build --- activerecord/lib/active_record/sanitization.rb | 3 ++- activerecord/test/cases/sanitize_test.rb | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index ef63949208..be62e41932 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -110,7 +110,8 @@ module ActiveRecord # Sanitizes a +string+ so that it is safe to use within a sql # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%" def sanitize_sql_like(string, escape_character = "\\") - string.gsub(/[\\_%]/) { |x| [escape_character, x].join } + pattern = Regexp.union(escape_character, "%", "_") + string.gsub(pattern) { |x| [escape_character, x].join } end # Accepts an array of conditions. The array has each value diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 18182efc46..c7cc214c3f 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -62,19 +62,20 @@ class SanitizeTest < ActiveRecord::TestCase def test_sanitize_sql_like_with_custom_escape_character assert_equal '100!%', Binary.send(:sanitize_sql_like, '100%', '!') assert_equal 'snake!_cased!_string', Binary.send(:sanitize_sql_like, 'snake_cased_string', '!') - assert_equal 'C:!\\Programs!\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint', '!') + assert_equal 'great!!', Binary.send(:sanitize_sql_like, 'great!', '!') + assert_equal 'C:\\Programs\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint', '!') assert_equal 'normal string 42', Binary.send(:sanitize_sql_like, 'normal string 42', '!') end def test_sanitize_sql_like_example_use_case searchable_post = Class.new(Post) do def self.search(term) - where("title LIKE ?", sanitize_sql_like(term)) + where("title LIKE ?", sanitize_sql_like(term, '!')) end end - assert_sql /LIKE '20\\% \\_reduction\\_'/ do - searchable_post.search("20% _reduction_").to_a + assert_sql /LIKE '20!% !_reduction!_!!'/ do + searchable_post.search("20% _reduction_!").to_a end end end -- cgit v1.2.3 From 4db2e1f78044b7f39119daa9589cc6b8ae48f726 Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Thu, 17 Apr 2014 12:10:20 -0400 Subject: Bring SQLite3Adpter init API closer to others --- .../lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 1 - activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 20eea208ec..d2ef83b047 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -187,7 +187,6 @@ module ActiveRecord include Arel::Visitors::BindVisitor end - # FIXME: Make the first parameter more similar for the two adapters def initialize(connection, logger, connection_options, config) super(connection, logger) @connection_options, @config = connection_options, config diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index cd1f7a16c6..2d5c47967d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -30,7 +30,7 @@ module ActiveRecord db.busy_timeout(ConnectionAdapters::SQLite3Adapter.type_cast_config_to_integer(config[:timeout])) if config[:timeout] - ConnectionAdapters::SQLite3Adapter.new(db, logger, config) + ConnectionAdapters::SQLite3Adapter.new(db, logger, nil, config) rescue Errno::ENOENT => error if error.message.include?("No such file or directory") raise ActiveRecord::NoDatabaseError.new(error.message, error) @@ -127,7 +127,7 @@ module ActiveRecord include Arel::Visitors::BindVisitor end - def initialize(connection, logger, config) + def initialize(connection, logger, connection_options, config) super(connection, logger) @active = nil -- cgit v1.2.3 From 1a7338d868d68ecef899fb8d81cf37852cc94a8c Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Thu, 17 Apr 2014 14:18:58 -0700 Subject: Optimize select_value, select_values, select_rows and dry up checking whether to exec with cache for Postgresql adapter Reduces creating unused objects, with the most dramatic reduction in select_values which used to map(&:first) an array of single element arrays. --- .../postgresql/database_statements.rb | 52 ++++++++++++++-------- .../connection_adapters/postgresql_adapter.rb | 8 ++++ 2 files changed, 41 insertions(+), 19 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index a5fb048b1e..168b08ba75 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -44,10 +44,32 @@ module ActiveRecord end end + def select_value(arel, name = nil, binds = []) + arel, binds = binds_from_relation arel, binds + sql = to_sql(arel, binds) + execute_and_clear(sql, name, binds) do |result| + result.getvalue(0, 0) if result.ntuples > 0 && result.nfields > 0 + end + end + + def select_values(arel, name = nil) + arel, binds = binds_from_relation arel, [] + sql = to_sql(arel, binds) + execute_and_clear(sql, name, binds) do |result| + if result.nfields > 0 + result.column_values(0) + else + [] + end + end + end + # Executes a SELECT query and returns an array of rows. Each row is an # array of field values. def select_rows(sql, name = nil, binds = []) - exec_query(sql, name, binds).rows + execute_and_clear(sql, name, binds) do |result| + result.values + end end # Executes an INSERT query and returns the new record's ID @@ -134,28 +156,20 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : - exec_cache(sql, name, binds) - - types = {} - fields = result.fields - fields.each_with_index do |fname, i| - ftype = result.ftype i - fmod = result.fmod i - types[fname] = get_oid_type(ftype, fmod, fname) + execute_and_clear(sql, name, binds) do |result| + types = {} + fields = result.fields + fields.each_with_index do |fname, i| + ftype = result.ftype i + fmod = result.fmod i + types[fname] = get_oid_type(ftype, fmod, fname) + end + ActiveRecord::Result.new(fields, result.values, types) end - - ret = ActiveRecord::Result.new(fields, result.values, types) - result.clear - return ret end def exec_delete(sql, name = 'SQL', binds = []) - result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : - exec_cache(sql, name, binds) - affected = result.cmd_tuples - result.clear - affected + execute_and_clear(sql, name, binds) {|result| result.cmd_tuples } end alias :exec_update :exec_delete diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0485093123..1f1bd342d1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -662,6 +662,14 @@ module ActiveRecord FEATURE_NOT_SUPPORTED = "0A000" #:nodoc: + def execute_and_clear(sql, name, binds) + result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : + exec_cache(sql, name, binds) + ret = yield result + result.clear + ret + end + def exec_no_cache(sql, name, binds) log(sql, name, binds) { @connection.async_exec(sql) } end -- cgit v1.2.3 From bcfa2bf958ebd9e381944bd93ee2f436a1edd375 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 13 Mar 2013 16:54:25 +0100 Subject: Singularize association names before camelization So that irregular multi-word pluralization rules have to be defined only for snake-case strings. --- activerecord/lib/active_record/reflection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 1724ea95b0..4fde6677be 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -449,9 +449,9 @@ module ActiveRecord end def derive_class_name - class_name = name.to_s.camelize + class_name = name.to_s class_name = class_name.singularize if collection? - class_name + class_name.camelize end def derive_foreign_key -- cgit v1.2.3 From c0b6e164ee6bbc7941d280ea629d70d400561668 Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Fri, 18 Apr 2014 01:11:47 -0400 Subject: Regression test for irregular inflection on has_many Also add a Changelog entry [related #9702] [fixes #8928] --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/test/cases/reflection_test.rb | 8 ++++++++ 2 files changed, 14 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 57a083c1df..0cba1009b6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fixed has_many association to make it support irregular inflections. + + Fixes #8928. + + *arthurnn*, *Javier Goizueta* + * Fixed a problem where count used with a grouping was not returning a Hash. Fixes #14721. diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index fed199f6e9..c085fcf161 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -87,6 +87,14 @@ class ReflectionTest < ActiveRecord::TestCase end end + def test_irregular_reflection_class_name + ActiveSupport::Inflector.inflections do |inflect| + inflect.irregular 'plural_irregular', 'plurales_irregulares' + end + reflection = AssociationReflection.new(:has_many, 'plurales_irregulares', nil, {}, ActiveRecord::Base) + assert_equal 'PluralIrregular', reflection.class_name + end + def test_aggregation_reflection reflection_for_address = AggregateReflection.new( :composed_of, :address, nil, { :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer -- cgit v1.2.3 From 03042b04764cdb6e49b92cff6462470906a6e9d9 Mon Sep 17 00:00:00 2001 From: Kuldeep Aggarwal Date: Fri, 18 Apr 2014 23:15:42 +0530 Subject: remove warning `warning: ambiguous first argument; put parentheses or even spaces` --- activerecord/test/cases/sanitize_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index c7cc214c3f..dca85fb5eb 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -74,7 +74,7 @@ class SanitizeTest < ActiveRecord::TestCase end end - assert_sql /LIKE '20!% !_reduction!_!!'/ do + assert_sql(/LIKE '20!% !_reduction!_!!'/) do searchable_post.search("20% _reduction_!").to_a end end -- cgit v1.2.3 From 5fe4e62807adc61c42c2d94ec36802f25554224a Mon Sep 17 00:00:00 2001 From: Kuldeep Aggarwal Date: Sat, 19 Apr 2014 00:01:01 +0530 Subject: `@destroyed` should always be set to `false` when an object is duped. --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/core.rb | 1 + activerecord/test/cases/persistence_test.rb | 16 ++++++++++++++++ 3 files changed, 21 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0cba1009b6..74f5666de0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* `@destroyed` should always be set to `false` when an object is duped. + + *Kuldeep Aggarwal* + * Fixed has_many association to make it support irregular inflections. Fixes #8928. diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 4e53f66005..d6df98a80f 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -267,6 +267,7 @@ module ActiveRecord @attributes_cache = {} @new_record = true + @destroyed = false super end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 046fe83e54..9209672ac5 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -233,6 +233,22 @@ class PersistenceTest < ActiveRecord::TestCase assert_nothing_raised { Minimalistic.create!(:id => 2) } end + def test_save_with_duping_of_destroyed_object + developer = Developer.create(name: "Kuldeep") + developer.destroy + new_developer = developer.dup + new_developer.save + assert new_developer.persisted? + end + + def test_dup_of_destroyed_object_is_not_destroyed + developer = Developer.create(name: "Kuldeep") + developer.destroy + new_developer = developer.dup + new_developer.save + assert_equal new_developer.destroyed?, false + end + def test_create_many topics = Topic.create([ { "title" => "first" }, { "title" => "second" }]) assert_equal 2, topics.size -- cgit v1.2.3