From ac623b8511d064419a1ae32d36d1c95c28aba01c Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 25 Feb 2017 12:38:09 +0200 Subject: Add test for method `#attributes` ActiveRecord::AttributeMethods#attributes Extracted from https://github.com/rails/rails/pull/28159 --- activerecord/test/cases/base_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 979a59f566..8efbc41da9 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -703,6 +703,17 @@ class BasicsTest < ActiveRecord::TestCase assert_nil topic.bonus_time end + def test_attributes + category = Category.new(name: "Ruby") + + expected_attributes = category.attribute_names.map do |attribute_name| + [attribute_name, category.public_send(attribute_name)] + end.to_h + + assert_instance_of Hash, category.attributes + assert_equal expected_attributes, category.attributes + end + def test_boolean b_nil = Boolean.create("value" => nil) nil_id = b_nil.id -- cgit v1.2.3 From 7b78cf99986cf06c5fbcabf5586bdd6ed322edf2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 21 Aug 2016 16:32:13 +0900 Subject: Avoid to handle polymorphic association for `AssociationQueryHandler` It should be handled by `PolymorphicArrayHandler` if polymorphic association. --- .../predicate_builder/association_query_handler.rb | 36 ++++++---------------- .../predicate_builder/polymorphic_array_handler.rb | 14 +++++++-- 2 files changed, 21 insertions(+), 29 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index 29860ec677..82da4689f0 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -3,12 +3,15 @@ module ActiveRecord class AssociationQueryHandler # :nodoc: def self.value_for(table, column, value) associated_table = table.associated_table(column) - klass = if associated_table.polymorphic_association? && ::Array === value && value.first.is_a?(Base) - PolymorphicArrayValue - else - AssociationQueryValue + if associated_table.polymorphic_association? + case value.is_a?(Array) ? value.first : value + when Base, Relation + value = [value] unless value.is_a?(Array) + klass = PolymorphicArrayValue + end end + klass ||= AssociationQueryValue klass.new(associated_table, value) end @@ -17,14 +20,8 @@ module ActiveRecord end def call(attribute, value) - queries = {} - table = value.associated_table - if value.base_class - queries[table.association_foreign_type.to_s] = value.base_class.name - end - - queries[table.association_foreign_key.to_s] = value.ids + queries = { table.association_foreign_key.to_s => value.ids } predicate_builder.build_from_hash(queries) end @@ -54,25 +51,10 @@ module ActiveRecord end end - def base_class - if associated_table.polymorphic_association? - @base_class ||= polymorphic_base_class_from_value - end - end - private def primary_key - associated_table.association_primary_key(base_class) - end - - def polymorphic_base_class_from_value - case value - when Relation - value.klass.base_class - when Base - value.class.base_class - end + associated_table.association_primary_key end def convert_to_id(value) diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb index 335124c952..21cc324e5e 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb @@ -48,11 +48,21 @@ module ActiveRecord end def base_class(value) - value.class.base_class + case value + when Base + value.class.base_class + when Relation + value.klass.base_class + end end def convert_to_id(value) - value._read_attribute(primary_key(value)) + case value + when Base + value._read_attribute(primary_key(value)) + when Relation + value.select(primary_key(value)) + end end end end -- cgit v1.2.3 From 56e2c9e1f09d80ec56e8dd9c5418a7f1d4e37c1f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 5 Apr 2017 19:14:56 +0900 Subject: Ignore AR tests of index comment when using Oracle --- activerecord/test/cases/comment_test.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 63f67a9a16..c23be52a6c 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -72,9 +72,11 @@ if ActiveRecord::Base.connection.supports_comments? end def test_add_index_with_comment_later - @connection.add_index :commenteds, :obvious, name: "idx_obvious", comment: "We need to see obvious comments" - index = @connection.indexes("commenteds").find { |idef| idef.name == "idx_obvious" } - assert_equal "We need to see obvious comments", index.comment + unless current_adapter?(:OracleAdapter) + @connection.add_index :commenteds, :obvious, name: "idx_obvious", comment: "We need to see obvious comments" + index = @connection.indexes("commenteds").find { |idef| idef.name == "idx_obvious" } + assert_equal "We need to see obvious comments", index.comment + end end def test_add_comment_to_column @@ -112,8 +114,10 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.string\s+"obvious"\n], output assert_match %r[t\.string\s+"content",\s+comment: "Whoa, content describes itself!"], output assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output - assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output - assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output + unless current_adapter?(:OracleAdapter) + assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output + assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output + end end def test_schema_dump_omits_blank_comments -- cgit v1.2.3 From 826e49cfbe9589e600e652c31eb62954dcc86061 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Wed, 5 Apr 2017 22:19:05 +0300 Subject: use formatted number as schema version --- activerecord/lib/active_record/schema_dumper.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 2bbfd01698..657bd43b86 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -47,8 +47,16 @@ module ActiveRecord @options = options end + # turns 20170404131909 into "2017_04_04131909" + def formatted_version + return "" unless @version + stringified = @version.to_s + return stringified unless stringified.length == 14 + stringified.insert(4, "_").insert(7, "_").insert(10, "_") + end + def header(stream) - define_params = @version ? "version: #{@version}" : "" + define_params = @version ? "version: #{formatted_version}" : "" stream.puts <
Date: Fri, 7 Apr 2017 08:40:52 +0900 Subject: Remove duplicated "test" prefix --- activerecord/test/cases/adapters/mysql2/boolean_test.rb | 4 ++-- activerecord/test/cases/cache_key_test.rb | 2 +- activerecord/test/cases/relation/mutation_test.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql2/boolean_test.rb b/activerecord/test/cases/adapters/mysql2/boolean_test.rb index 2fa39282fb..58698d59db 100644 --- a/activerecord/test/cases/adapters/mysql2/boolean_test.rb +++ b/activerecord/test/cases/adapters/mysql2/boolean_test.rb @@ -38,7 +38,7 @@ class Mysql2BooleanTest < ActiveRecord::Mysql2TestCase assert_equal :string, string_column.type end - test "test type casting with emulated booleans" do + test "type casting with emulated booleans" do emulate_booleans true boolean = BooleanType.create!(archived: true, published: true) @@ -55,7 +55,7 @@ class Mysql2BooleanTest < ActiveRecord::Mysql2TestCase assert_equal 0, @connection.type_cast(false) end - test "test type casting without emulated booleans" do + test "type casting without emulated booleans" do emulate_booleans false boolean = BooleanType.create!(archived: true, published: true) diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb index bb2829b3c1..2c6a38ec35 100644 --- a/activerecord/test/cases/cache_key_test.rb +++ b/activerecord/test/cases/cache_key_test.rb @@ -15,7 +15,7 @@ module ActiveRecord @connection.drop_table :cache_mes, if_exists: true end - test "test_cache_key_format_is_not_too_precise" do + test "cache_key format is not too precise" do record = CacheMe.create key = record.cache_key diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 4f92f71a09..11ef0d8743 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -143,7 +143,7 @@ module ActiveRecord assert_equal({ foo: "bar" }, relation.create_with_value) end - test "test_merge!" do + test "merge!" do assert relation.merge!(select: :foo).equal?(relation) assert_equal [:foo], relation.select_values end -- cgit v1.2.3 From 06f45435da941d713afefd3140421d1ced7abbdc Mon Sep 17 00:00:00 2001 From: Kevin McPhillips Date: Thu, 6 Apr 2017 21:43:58 -0400 Subject: Passing in no arguments to the dynamic fixture accessor method returns all fixtures, not an empty array. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/fixtures.rb | 29 +++++++++++++++++++++++++---- activerecord/test/cases/fixtures_test.rb | 18 ++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 30d580b9e3..9e3b686bbb 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1 +1,7 @@ +* When calling the dynamic fixture accessor method with no arguments it now returns all fixtures of this type. + Previously this method always returned an empty array. + + *Kevin McPhillips* + + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index e79167d568..c19216702c 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -70,13 +70,32 @@ module ActiveRecord # test. To ensure consistent data, the environment deletes the fixtures before running the load. # # In addition to being available in the database, the fixture's data may also be accessed by - # using a special dynamic method, which has the same name as the model, and accepts the - # name of the fixture to instantiate: + # using a special dynamic method, which has the same name as the model. # - # test "find" do + # Passing in a fixture name to this dynamic method returns the fixture matching this name: + # + # test "find one" do # assert_equal "Ruby on Rails", web_sites(:rubyonrails).name # end # + # Passing in multiple fixture names returns all fixtures matching these names: + # + # test "find all by name" do + # assert_equal 2, web_sites(:rubyonrails, :google).length + # end + # + # Passing in no arguments returns all fixtures: + # + # test "find all" do + # assert_equal 2, web_sites.length + # end + # + # Passing in any fixture name that does not exist will raise StandardError: + # + # test "find by name that does not exist" do + # assert_raise(StandardError) { web_sites(:reddit) } + # end + # # Alternatively, you may enable auto-instantiation of the fixture data. For instance, take the # following tests: # @@ -909,6 +928,8 @@ module ActiveRecord define_method(accessor_name) do |*fixture_names| force_reload = fixture_names.pop if fixture_names.last == true || fixture_names.last == :reload + return_single_record = fixture_names.size == 1 + fixture_names = @loaded_fixtures[fs_name].fixtures.keys if fixture_names.empty? @fixture_cache[fs_name] ||= {} @@ -923,7 +944,7 @@ module ActiveRecord end end - instances.size == 1 ? instances.first : instances + return_single_record ? instances.first : instances end private accessor_name end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 3720b0cc1a..a0a6d3c7ef 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -95,6 +95,24 @@ class FixturesTest < ActiveRecord::TestCase assert_nil(topics["second"]["author_email_address"]) end + def test_no_args_returns_all + all_topics = topics + assert_equal 5, all_topics.length + assert_equal "The First Topic", all_topics.first["title"] + assert_equal 5, all_topics.last.id + end + + def test_no_args_record_returns_all_without_array + all_binaries = binaries + assert_kind_of(Array, all_binaries) + assert_equal 1, binaries.length + end + + def test_nil_raises + assert_raise(StandardError) { topics(nil) } + assert_raise(StandardError) { topics([nil]) } + end + def test_inserts create_fixtures("topics") first_row = ActiveRecord::Base.connection.select_one("SELECT * FROM topics WHERE author_name = 'David'") -- cgit v1.2.3 From a76f5189f6cec4b3e6d9035e2b55dcda6050dfdb Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 8 Apr 2017 22:29:19 +0900 Subject: More exercise `exists?` tests --- activerecord/test/cases/finder_test.rb | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 89d8a8bdca..4fa664697c 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -202,11 +202,24 @@ class FinderTest < ActiveRecord::TestCase assert_equal true, Topic.first.replies.exists? end - # ensures +exists?+ runs valid SQL by excluding order value - def test_exists_with_order + # Ensure +exists?+ runs without an error by excluding distinct value. + # See https://github.com/rails/rails/pull/26981. + def test_exists_with_order_and_distinct assert_equal true, Topic.order(:id).distinct.exists? end + def test_exists_with_joins + assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists? + end + + def test_exists_with_left_joins + assert_equal true, Topic.left_joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists? + end + + def test_exists_with_eager_load + assert_equal true, Topic.eager_load(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists? + end + def test_exists_with_includes_limit_and_empty_result assert_equal false, Topic.includes(:replies).limit(0).exists? assert_equal false, Topic.includes(:replies).limit(1).where("0 = 1").exists? @@ -236,9 +249,9 @@ class FinderTest < ActiveRecord::TestCase def test_exists_with_aggregate_having_three_mappings_with_one_difference existing_address = customers(:david).address - assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city, existing_address.country + "1")) - assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city + "1", existing_address.country)) - assert_equal false, Customer.exists?(address: Address.new(existing_address.street + "1", existing_address.city, existing_address.country)) + assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city, existing_address.country + "1")) + assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city + "1", existing_address.country)) + assert_equal false, Customer.exists?(address: Address.new(existing_address.street + "1", existing_address.city, existing_address.country)) end def test_exists_does_not_instantiate_records -- cgit v1.2.3 From 0da696a5e3cee87a996a020b664caa1eabd66220 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 8 Apr 2017 22:42:31 +0900 Subject: Extract `construct_relation_for_exists` in `FinderMethods` To ease to customize a relation for `exists?`. --- .../lib/active_record/relation/finder_methods.rb | 27 ++++++++++++---------- 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 5d24f5f5ca..14c2ddd85b 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -312,16 +312,7 @@ module ActiveRecord relation = apply_join_dependency(self, construct_join_dependency(eager_loading: false)) return false if ActiveRecord::NullRelation === relation - relation = relation.except(:select, :distinct).select(ONE_AS_ONE).limit(1) - - case conditions - when Array, Hash - relation = relation.where(conditions) - else - unless conditions == :none - relation = relation.where(primary_key => conditions) - end - end + relation = construct_relation_for_exists(relation, conditions) connection.select_value(relation, "#{name} Exists", relation.bound_attributes) ? true : false rescue ::RangeError @@ -391,6 +382,19 @@ module ActiveRecord end end + def construct_relation_for_exists(relation, conditions) + relation = relation.except(:select, :distinct)._select!(ONE_AS_ONE).limit!(1) + + case conditions + when Array, Hash + relation.where!(conditions) + else + relation.where!(primary_key => conditions) unless conditions == :none + end + + relation + end + def construct_join_dependency(joins = [], eager_loading: true) including = eager_load_values + includes_values ActiveRecord::Associations::JoinDependency.new(@klass, including, joins, eager_loading: eager_loading) @@ -401,8 +405,7 @@ module ActiveRecord end def apply_join_dependency(relation, join_dependency) - relation = relation.except(:includes, :eager_load, :preload) - relation = relation.joins join_dependency + relation = relation.except(:includes, :eager_load, :preload).joins!(join_dependency) if using_limitable_reflections?(join_dependency.reflections) relation -- cgit v1.2.3 From 1d05e64b6dc5ba4ed1e21dd2975dcd7faf36795d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 9 Apr 2017 14:53:21 +0900 Subject: Tweaks #28678 * Fix the comment on `formatted_version` * Extract `define_params` * Remove duplicated guard clause for `@version` --- activerecord/lib/active_record/schema_dumper.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 657bd43b86..94d63765c9 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -47,17 +47,18 @@ module ActiveRecord @options = options end - # turns 20170404131909 into "2017_04_04131909" + # turns 20170404131909 into "2017_04_04_131909" def formatted_version - return "" unless @version stringified = @version.to_s return stringified unless stringified.length == 14 stringified.insert(4, "_").insert(7, "_").insert(10, "_") end - def header(stream) - define_params = @version ? "version: #{formatted_version}" : "" + def define_params + @version ? "version: #{formatted_version}" : "" + end + def header(stream) stream.puts <
Date: Sun, 9 Apr 2017 16:59:26 +0900 Subject: Expose `queries` for `AssociationQueryValue` and `PolymorphicArrayValue` --- .../predicate_builder/association_query_handler.rb | 25 +++++++++++----------- .../predicate_builder/polymorphic_array_handler.rb | 21 ++++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index 82da4689f0..9141d9c537 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -20,9 +20,7 @@ module ActiveRecord end def call(attribute, value) - table = value.associated_table - queries = { table.association_foreign_key.to_s => value.ids } - predicate_builder.build_from_hash(queries) + predicate_builder.build_from_hash(value.queries) end # TODO Change this to private once we've dropped Ruby 2.2 support. @@ -40,18 +38,21 @@ module ActiveRecord @value = value end - def ids - case value - when Relation - value.select(primary_key) - when Array - value.map { |v| convert_to_id(v) } - else - convert_to_id(value) - end + def queries + { associated_table.association_foreign_key.to_s => ids } end private + def ids + case value + when Relation + value.select_values.empty? ? value.select(primary_key) : value + when Array + value.map { |v| convert_to_id(v) } + else + convert_to_id(value) + end + end def primary_key associated_table.association_primary_key diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb index 21cc324e5e..c2f136256b 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb @@ -6,12 +6,7 @@ module ActiveRecord end def call(attribute, value) - table = value.associated_table - queries = value.type_to_ids_mapping.map do |type, ids| - { table.association_foreign_type.to_s => type, table.association_foreign_key.to_s => ids } - end - - predicates = queries.map { |query| predicate_builder.build_from_hash(query) } + predicates = value.queries.map { |query| predicate_builder.build_from_hash(query) } if predicates.size > 1 type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) } @@ -36,12 +31,20 @@ module ActiveRecord @values = values end - def type_to_ids_mapping - default_hash = Hash.new { |hsh, key| hsh[key] = [] } - values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + def queries + type_to_ids_mapping.map do |type, ids| + { + associated_table.association_foreign_type.to_s => type, + associated_table.association_foreign_key.to_s => ids + } + end end private + def type_to_ids_mapping + default_hash = Hash.new { |hsh, key| hsh[key] = [] } + values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + end def primary_key(value) associated_table.association_primary_key(base_class(value)) -- cgit v1.2.3 From baf6072e4550cf2cf5e52240d0192a56dbe8e949 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 9 Apr 2017 17:25:43 +0900 Subject: Convert `AssociationQueryValue` to PORO queries --- .../active_record/relation/predicate_builder.rb | 28 ++++++++++++++++---- .../predicate_builder/association_query_handler.rb | 30 ---------------------- 2 files changed, 23 insertions(+), 35 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index fca914aedd..58d30b801c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -20,7 +20,6 @@ module ActiveRecord register_handler(RangeHandler::RangeWithBinds, RangeHandler.new) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) - register_handler(AssociationQueryValue, AssociationQueryHandler.new(self)) register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self)) end @@ -83,11 +82,10 @@ module ActiveRecord end def create_binds_for_hash(attributes) - result = attributes.dup + result = {} binds = [] attributes.each do |column_name, value| - binds.concat(value.bound_attributes) if value.is_a?(Relation) case when value.is_a?(Hash) && !table.has_column?(column_name) attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value) @@ -99,7 +97,24 @@ module ActiveRecord # # For polymorphic relationships, find the foreign key and type: # PriceEstimate.where(estimate_of: treasure) - result[column_name] = AssociationQueryHandler.value_for(table, column_name, value) + associated_table = table.associated_table(column_name) + if associated_table.polymorphic_association? + case value.is_a?(Array) ? value.first : value + when Base, Relation + binds.concat(value.bound_attributes) if value.is_a?(Relation) + value = [value] unless value.is_a?(Array) + klass = PolymorphicArrayValue + end + end + + if klass + result[column_name] = klass.new(associated_table, value) + else + queries = AssociationQueryValue.new(associated_table, value).queries + attrs, bvs = create_binds_for_hash(queries) + result.merge!(attrs) + binds.concat(bvs) + end when value.is_a?(Range) && !table.type(column_name).respond_to?(:subtype) first = value.begin last = value.end @@ -115,9 +130,12 @@ module ActiveRecord result[column_name] = RangeHandler::RangeWithBinds.new(first, last, value.exclude_end?) else if can_be_bound?(column_name, value) - result[column_name] = Arel::Nodes::BindParam.new binds << build_bind_param(column_name, value) + value = Arel::Nodes::BindParam.new + elsif value.is_a?(Relation) + binds.concat(value.bound_attributes) end + result[column_name] = value end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index 9141d9c537..cc39aff91a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -1,35 +1,5 @@ module ActiveRecord class PredicateBuilder - class AssociationQueryHandler # :nodoc: - def self.value_for(table, column, value) - associated_table = table.associated_table(column) - if associated_table.polymorphic_association? - case value.is_a?(Array) ? value.first : value - when Base, Relation - value = [value] unless value.is_a?(Array) - klass = PolymorphicArrayValue - end - end - - klass ||= AssociationQueryValue - klass.new(associated_table, value) - end - - def initialize(predicate_builder) - @predicate_builder = predicate_builder - end - - def call(attribute, value) - predicate_builder.build_from_hash(value.queries) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :predicate_builder - end - class AssociationQueryValue # :nodoc: attr_reader :associated_table, :value -- cgit v1.2.3 From 8170bcd99ad3ee4ac4ddf9e28a7c2a5fb93f1b0c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 9 Apr 2017 19:24:33 +0900 Subject: Convert `PolymorphicArrayValue` to PORO queries --- .../active_record/relation/predicate_builder.rb | 8 ++++--- .../relation/predicate_builder/array_handler.rb | 14 +++++++++++- .../predicate_builder/polymorphic_array_handler.rb | 25 +--------------------- 3 files changed, 19 insertions(+), 28 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 58d30b801c..f2e719363c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -20,7 +20,6 @@ module ActiveRecord register_handler(RangeHandler::RangeWithBinds, RangeHandler.new) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) - register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self)) end def build_from_hash(attributes) @@ -101,14 +100,17 @@ module ActiveRecord if associated_table.polymorphic_association? case value.is_a?(Array) ? value.first : value when Base, Relation - binds.concat(value.bound_attributes) if value.is_a?(Relation) value = [value] unless value.is_a?(Array) klass = PolymorphicArrayValue end end if klass - result[column_name] = klass.new(associated_table, value) + result[column_name] = klass.new(associated_table, value).queries.map do |query| + attrs, bvs = create_binds_for_hash(query) + binds.concat(bvs) + attrs + end else queries = AssociationQueryValue.new(associated_table, value).queries attrs, bvs = create_binds_for_hash(queries) diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 88b6c37d43..54e9910598 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -10,6 +10,7 @@ module ActiveRecord nils, values = values.partition(&:nil?) return attribute.in([]) if values.empty? && nils.empty? + return queries_predicates(values) if nils.empty? && values.all? { |v| v.is_a?(Hash) } ranges, values = values.partition { |v| v.is_a?(Range) } @@ -26,7 +27,7 @@ module ActiveRecord array_predicates = ranges.map { |range| predicate_builder.build(attribute, range) } array_predicates.unshift(values_predicate) - array_predicates.inject { |composite, predicate| composite.or(predicate) } + array_predicates.inject(&:or) end # TODO Change this to private once we've dropped Ruby 2.2 support. @@ -40,6 +41,17 @@ module ActiveRecord other end end + + private + def queries_predicates(queries) + if queries.size > 1 + queries.map do |query| + Arel::Nodes::And.new(predicate_builder.build_from_hash(query)) + end.inject(&:or) + else + predicate_builder.build_from_hash(queries.first) + end + end end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb index c2f136256b..9bb2f8c8dc 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb @@ -1,28 +1,5 @@ module ActiveRecord class PredicateBuilder - class PolymorphicArrayHandler # :nodoc: - def initialize(predicate_builder) - @predicate_builder = predicate_builder - end - - def call(attribute, value) - predicates = value.queries.map { |query| predicate_builder.build_from_hash(query) } - - if predicates.size > 1 - type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) } - type_and_ids_predicates.inject(&:or) - else - predicates.first - end - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :predicate_builder - end - class PolymorphicArrayValue # :nodoc: attr_reader :associated_table, :values @@ -35,7 +12,7 @@ module ActiveRecord type_to_ids_mapping.map do |type, ids| { associated_table.association_foreign_type.to_s => type, - associated_table.association_foreign_key.to_s => ids + associated_table.association_foreign_key.to_s => ids.size > 1 ? ids : ids.first } end end -- cgit v1.2.3 From 24ac36be7150f97ac0a61cf7cbe7d212097ef1a6 Mon Sep 17 00:00:00 2001 From: Boris Slobodin Date: Sun, 9 Apr 2017 16:05:13 -0700 Subject: exclude ORDER BY clause for exists? (#28699) --- activerecord/lib/active_record/relation/finder_methods.rb | 2 +- activerecord/test/cases/finder_test.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 14c2ddd85b..a1459c87c6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -383,7 +383,7 @@ module ActiveRecord end def construct_relation_for_exists(relation, conditions) - relation = relation.except(:select, :distinct)._select!(ONE_AS_ONE).limit!(1) + relation = relation.except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) case conditions when Array, Hash diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 4fa664697c..a7b6333010 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -208,6 +208,11 @@ class FinderTest < ActiveRecord::TestCase assert_equal true, Topic.order(:id).distinct.exists? end + # Ensure +exists?+ runs without an error by excluding order value. + def test_exists_with_order + assert_equal true, Topic.order("invalid sql here").exists? + end + def test_exists_with_joins assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists? end -- cgit v1.2.3 From e4c197c7698e204d0c74a2ece20adf831c2f9a8d Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Tue, 11 Apr 2017 08:10:24 +0930 Subject: Add comprehensive locking around DB transactions Transactional-fixture using tests with racing threads and inter-thread synchronisation inside transaction blocks will now deadlock... but without this, they would just crash. In 5.0, the threads didn't share a connection at all, so it would've worked... but with the main thread inside the fixture transaction, they wouldn't've been able to see each other. So: as far as I can tell, the set of operations this "breaks" never had a compelling use case. Meanwhile, it provides an increased level of coherency to the operational feel of transactional fixtures. If this does cause anyone problems, they're probably best off disabling transactional fixtures on the affected tests, and managing transactions themselves. --- .../connection_adapters/abstract/transaction.rb | 88 ++++++++++++---------- .../connection_adapters/abstract_adapter.rb | 2 +- .../connection_adapters/postgresql_adapter.rb | 32 +++++--- 3 files changed, 70 insertions(+), 52 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 6bb072dd73..19b7821494 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -149,57 +149,67 @@ module ActiveRecord end def begin_transaction(options = {}) - run_commit_callbacks = !current_transaction.joinable? - transaction = - if @stack.empty? - RealTransaction.new(@connection, options, run_commit_callbacks: run_commit_callbacks) - else - SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options, - run_commit_callbacks: run_commit_callbacks) - end + @connection.lock.synchronize do + run_commit_callbacks = !current_transaction.joinable? + transaction = + if @stack.empty? + RealTransaction.new(@connection, options, run_commit_callbacks: run_commit_callbacks) + else + SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options, + run_commit_callbacks: run_commit_callbacks) + end - @stack.push(transaction) - transaction + @stack.push(transaction) + transaction + end end def commit_transaction - transaction = @stack.last + @connection.lock.synchronize do + transaction = @stack.last - begin - transaction.before_commit_records - ensure - @stack.pop - end + begin + transaction.before_commit_records + ensure + @stack.pop + end - transaction.commit - transaction.commit_records + transaction.commit + transaction.commit_records + end end def rollback_transaction(transaction = nil) - transaction ||= @stack.pop - transaction.rollback - transaction.rollback_records + @connection.lock.synchronize do + transaction ||= @stack.pop + transaction.rollback + transaction.rollback_records + end end def within_new_transaction(options = {}) - transaction = begin_transaction options - yield - rescue Exception => error - if transaction - rollback_transaction - after_failure_actions(transaction, error) - end - raise - ensure - unless error - if Thread.current.status == "aborting" - rollback_transaction if transaction - else - begin - commit_transaction - rescue Exception - rollback_transaction(transaction) unless transaction.state.completed? - raise + @connection.lock.synchronize do + begin + transaction = begin_transaction options + yield + rescue Exception => error + if transaction + rollback_transaction + after_failure_actions(transaction, error) + end + raise + ensure + unless error + if Thread.current.status == "aborting" + rollback_transaction if transaction + else + begin + commit_transaction + rescue Exception + rollback_transaction(transaction) unless transaction.state.completed? + raise + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 96083e6519..85d6fbe8b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -74,7 +74,7 @@ module ActiveRecord SIMPLE_INT = /\A\d+\z/ attr_accessor :visitor, :pool - attr_reader :schema_cache, :owner, :logger, :prepared_statements + attr_reader :schema_cache, :owner, :logger, :prepared_statements, :lock alias :in_use? :owner def self.type_cast_config_to_integer(config) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0ad114165e..7ae9bd9a5f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -239,7 +239,9 @@ module ActiveRecord # Is this connection alive and ready for queries? def active? - @connection.query "SELECT 1" + @lock.synchronize do + @connection.query "SELECT 1" + end true rescue PG::Error false @@ -247,26 +249,32 @@ module ActiveRecord # Close then reopen the connection. def reconnect! - super - @connection.reset - configure_connection + @lock.synchronize do + super + @connection.reset + configure_connection + end end def reset! - clear_cache! - reset_transaction - unless @connection.transaction_status == ::PG::PQTRANS_IDLE - @connection.query "ROLLBACK" + @lock.synchronize do + clear_cache! + reset_transaction + unless @connection.transaction_status == ::PG::PQTRANS_IDLE + @connection.query "ROLLBACK" + end + @connection.query "DISCARD ALL" + configure_connection end - @connection.query "DISCARD ALL" - configure_connection end # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! - super - @connection.close rescue nil + @lock.synchronize do + super + @connection.close rescue nil + end end def native_database_types #:nodoc: -- cgit v1.2.3 From eae0ab97b2f8fdb3f7d62fa14642c54421edc4ff Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 10 Apr 2017 02:59:23 +0900 Subject: `AssociationQueryValue#queries` returns an array for more concise association handling --- .../lib/active_record/relation/predicate_builder.rb | 19 ++++++------------- .../predicate_builder/association_query_handler.rb | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index f2e719363c..133b3d837c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -81,7 +81,7 @@ module ActiveRecord end def create_binds_for_hash(attributes) - result = {} + result = attributes.dup binds = [] attributes.each do |column_name, value| @@ -105,17 +105,11 @@ module ActiveRecord end end - if klass - result[column_name] = klass.new(associated_table, value).queries.map do |query| - attrs, bvs = create_binds_for_hash(query) - binds.concat(bvs) - attrs - end - else - queries = AssociationQueryValue.new(associated_table, value).queries - attrs, bvs = create_binds_for_hash(queries) - result.merge!(attrs) + klass ||= AssociationQueryValue + result[column_name] = klass.new(associated_table, value).queries.map do |query| + attrs, bvs = create_binds_for_hash(query) binds.concat(bvs) + attrs end when value.is_a?(Range) && !table.type(column_name).respond_to?(:subtype) first = value.begin @@ -132,12 +126,11 @@ module ActiveRecord result[column_name] = RangeHandler::RangeWithBinds.new(first, last, value.exclude_end?) else if can_be_bound?(column_name, value) + result[column_name] = Arel::Nodes::BindParam.new binds << build_bind_param(column_name, value) - value = Arel::Nodes::BindParam.new elsif value.is_a?(Relation) binds.concat(value.bound_attributes) end - result[column_name] = value end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index cc39aff91a..2fe0f81cab 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -9,7 +9,7 @@ module ActiveRecord end def queries - { associated_table.association_foreign_key.to_s => ids } + [associated_table.association_foreign_key.to_s => ids] end private -- cgit v1.2.3 From 66cc3c7aa7b3aa2593cf10df728ad5d8ce71adcd Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 10 Apr 2017 16:05:13 +0900 Subject: Fix an AR test of relations_test when using Oracle --- activerecord/test/cases/relations_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 856469c710..fcf68b0f2a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1902,7 +1902,7 @@ class RelationTest < ActiveRecord::TestCase end test "relations don't load all records in #inspect" do - assert_sql(/LIMIT/) do + assert_sql(/LIMIT|ROWNUM <=|FETCH FIRST/) do Post.all.inspect end end -- cgit v1.2.3 From b5eb3215a68f94bb8cb20739366232c415744b83 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 4 Apr 2017 20:23:51 +0300 Subject: Fix inconsistency with changed attributes when overriding AR attribute reader --- activerecord/CHANGELOG.md | 4 ++++ .../lib/active_record/attribute_methods/dirty.rb | 4 ++++ activerecord/test/cases/dirty_test.rb | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 9e3b686bbb..ffe588b3c5 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix inconsistency with changed attributes when overriding AR attribute reader. + + *bogdanvlviv* + * When calling the dynamic fixture accessor method with no arguments it now returns all fixtures of this type. Previously this method always returned an empty array. diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index bd5003d63a..63978cfb3e 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -328,6 +328,10 @@ module ActiveRecord def clear_changed_attributes_cache remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) end + + def _attributes(attr) + _read_attribute(attr) + end end end end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index f9eccfbda1..15d7a32e21 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -671,6 +671,25 @@ class DirtyTest < ActiveRecord::TestCase assert binary.changed? end + test "changes is correct if override attribute reader" do + pirate = Pirate.create!(catchphrase: "arrrr") + def pirate.catchphrase + super.upcase + end + + new_catchphrase = "arrrr matey!" + + pirate.catchphrase = new_catchphrase + assert pirate.catchphrase_changed? + + expected_changes = { + "catchphrase" => ["arrrr", new_catchphrase] + } + + assert_equal new_catchphrase.upcase, pirate.catchphrase + assert_equal expected_changes, pirate.changes + end + test "attribute_changed? doesn't compute in-place changes for unrelated attributes" do test_type_class = Class.new(ActiveRecord::Type::Value) do define_method(:changed_in_place?) do |*| -- cgit v1.2.3 From 7384771dd0652ec4d82c0ad16522a87102316aee Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Wed, 12 Apr 2017 22:39:15 +0930 Subject: Use a query that's compatible with PostgreSQL 9.2 Also, explicitly apply the order: generate_subscripts is unlikely to start returning values out of order, but we should still be clear about what we want. --- .../postgresql/schema_statements.rb | 18 +++++++++++------- .../test/cases/adapters/postgresql/uuid_test.rb | 10 +++++++--- activerecord/test/cases/migration/rename_table_test.rb | 2 +- activerecord/test/cases/view_test.rb | 4 +++- activerecord/test/schema/postgresql_specific_schema.rb | 8 +++++--- 5 files changed, 27 insertions(+), 15 deletions(-) (limited to 'activerecord') 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 1d439acb07..02a6da2f71 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -344,13 +344,17 @@ module ActiveRecord def primary_keys(table_name) # :nodoc: select_values(<<-SQL.strip_heredoc, "SCHEMA") - SELECT a.attname FROM pg_index i - CROSS JOIN generate_subscripts(i.indkey, 1) k - JOIN pg_attribute a - ON a.attrelid = i.indrelid - AND a.attnum = i.indkey[k] - WHERE i.indrelid = #{quote(quote_table_name(table_name))}::regclass - AND i.indisprimary + SELECT a.attname + FROM ( + SELECT indrelid, indkey, generate_subscripts(indkey, 1) idx + FROM pg_index + WHERE indrelid = #{quote(quote_table_name(table_name))}::regclass + AND indisprimary + ) i + JOIN pg_attribute a + ON a.attrelid = i.indrelid + AND a.attnum = i.indkey[i.idx] + ORDER BY i.idx SQL end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 6aa6a79705..52e4a38cae 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -13,6 +13,10 @@ module PostgresqlUUIDHelper def uuid_function connection.supports_pgcrypto_uuid? ? "gen_random_uuid()" : "uuid_generate_v4()" end + + def uuid_default + connection.supports_pgcrypto_uuid? ? {} : { default: uuid_function } + end end class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase @@ -178,7 +182,7 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase t.uuid "other_uuid_2", default: "my_uuid_generator()" end - connection.create_table("pg_uuids_3", id: :uuid) do |t| + connection.create_table("pg_uuids_3", id: :uuid, **uuid_default) do |t| t.string "name" end end @@ -320,10 +324,10 @@ class PostgresqlUUIDTestInverseOf < ActiveRecord::PostgreSQLTestCase setup do connection.transaction do - connection.create_table("pg_uuid_posts", id: :uuid) do |t| + connection.create_table("pg_uuid_posts", id: :uuid, **uuid_default) do |t| t.string "title" end - connection.create_table("pg_uuid_comments", id: :uuid) do |t| + connection.create_table("pg_uuid_comments", id: :uuid, **uuid_default) do |t| t.references :uuid_post, type: :uuid t.string "content" end diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index 19588d28a2..7bcabd0cc6 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -80,7 +80,7 @@ module ActiveRecord end def test_renaming_table_doesnt_attempt_to_rename_non_existent_sequences - connection.create_table :cats, id: :uuid + connection.create_table :cats, id: :uuid, default: "uuid_generate_v4()" assert_nothing_raised { rename_table :cats, :felines } assert connection.table_exists? :felines ensure diff --git a/activerecord/test/cases/view_test.rb b/activerecord/test/cases/view_test.rb index 07288568e8..1d21a2454f 100644 --- a/activerecord/test/cases/view_test.rb +++ b/activerecord/test/cases/view_test.rb @@ -154,7 +154,9 @@ if ActiveRecord::Base.connection.supports_views? end # sqlite dose not support CREATE, INSERT, and DELETE for VIEW - if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter, :SQLServerAdapter) + if current_adapter?(:Mysql2Adapter, :SQLServerAdapter) || + current_adapter?(:PostgreSQLAdapter) && ActiveRecord::Base.connection.postgresql_version >= 90300 + class UpdateableViewTest < ActiveRecord::TestCase self.use_transactional_tests = false fixtures :books diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 860c63b27c..e56e8fa36a 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -3,11 +3,13 @@ ActiveRecord::Schema.define do enable_extension!("uuid-ossp", ActiveRecord::Base.connection) enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? - create_table :uuid_parents, id: :uuid, force: true do |t| + uuid_default = connection.supports_pgcrypto_uuid? ? {} : { default: "uuid_generate_v4()" } + + create_table :uuid_parents, id: :uuid, force: true, **uuid_default do |t| t.string :name end - create_table :uuid_children, id: :uuid, force: true do |t| + create_table :uuid_children, id: :uuid, force: true, **uuid_default do |t| t.string :name t.uuid :uuid_parent_id end @@ -102,7 +104,7 @@ _SQL end create_table :uuid_items, force: true, id: false do |t| - t.uuid :uuid, primary_key: true + t.uuid :uuid, primary_key: true, **uuid_default t.string :title end end -- cgit v1.2.3 From faca40dfd4032bbe2373210255eb7aa1c6527503 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 13 Apr 2017 07:15:04 +0900 Subject: :scissors: [ci skip] --- activerecord/lib/active_record/associations.rb | 2 +- activerecord/lib/active_record/dynamic_matchers.rb | 1 - activerecord/test/cases/coders/yaml_column_test.rb | 1 - activerecord/test/cases/integration_test.rb | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6efa448d49..e52c2004f3 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1649,7 +1649,7 @@ module ActiveRecord # you don't want to have association presence validated, use optional: true. # [:default] # Provide a callable (i.e. proc or lambda) to specify that the association should - # be initialized with a particular record before validation. + # be initialized with a particular record before validation. # # Option examples: # belongs_to :firm, foreign_key: "client_of" diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index 08d42f3dd4..d9912cfb22 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -1,4 +1,3 @@ - module ActiveRecord module DynamicMatchers #:nodoc: def respond_to_missing?(name, include_private = false) diff --git a/activerecord/test/cases/coders/yaml_column_test.rb b/activerecord/test/cases/coders/yaml_column_test.rb index 59ef389326..a26a72712d 100644 --- a/activerecord/test/cases/coders/yaml_column_test.rb +++ b/activerecord/test/cases/coders/yaml_column_test.rb @@ -1,4 +1,3 @@ - require "cases/helper" module ActiveRecord diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index d7aa091623..0678bb714f 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -1,4 +1,3 @@ - require "cases/helper" require "models/company" require "models/developer" -- cgit v1.2.3 From 58d52171bb8c7a5589bb13ce1bab81d635ae9cf0 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 13 Apr 2017 07:56:17 +0900 Subject: Add a test case for #20802 The issue #20802 has been fixed in cc0b566. Closes #20802. --- activerecord/test/cases/relation/where_test.rb | 5 +++++ activerecord/test/models/essay.rb | 1 + 2 files changed, 6 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index c7b2ac90fb..cbc466d6b8 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -289,6 +289,11 @@ module ActiveRecord assert_equal essays(:david_modest_proposal), essay end + def test_where_on_association_with_select_relation + essay = Essay.where(author: Author.where(name: "David").select(:name)).take + assert_equal essays(:david_modest_proposal), essay + end + def test_where_with_strong_parameters protected_params = Class.new do attr_reader :permitted diff --git a/activerecord/test/models/essay.rb b/activerecord/test/models/essay.rb index 13267fbc21..1f9772870e 100644 --- a/activerecord/test/models/essay.rb +++ b/activerecord/test/models/essay.rb @@ -1,4 +1,5 @@ class Essay < ActiveRecord::Base + belongs_to :author belongs_to :writer, primary_key: :name, polymorphic: true belongs_to :category, primary_key: :name has_one :owner, primary_key: :name -- cgit v1.2.3 From 83d6cc4815debe2086073387ecbf38c0f47cfec5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 14 Apr 2017 10:46:07 -0700 Subject: Move around AR::Dirty and fix _attribute method We already have a _read_attribute method that can get the value we need from the model. Lets define that method in AM::Dirty and use the existing one from AR::Dirty rather than introducing a new method. --- .../lib/active_record/attribute_methods/dirty.rb | 4 ---- activerecord/test/cases/dirty_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 63978cfb3e..bd5003d63a 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -328,10 +328,6 @@ module ActiveRecord def clear_changed_attributes_cache remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) end - - def _attributes(attr) - _read_attribute(attr) - end end end end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 15d7a32e21..721861975a 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -671,6 +671,28 @@ class DirtyTest < ActiveRecord::TestCase assert binary.changed? end + test "changes is correct for subclass" do + foo = Class.new(Pirate) do + def catchphrase + super.upcase + end + end + + pirate = foo.create!(catchphrase: "arrrr") + + new_catchphrase = "arrrr matey!" + + pirate.catchphrase = new_catchphrase + assert pirate.catchphrase_changed? + + expected_changes = { + "catchphrase" => ["arrrr", new_catchphrase] + } + + assert_equal new_catchphrase.upcase, pirate.catchphrase + assert_equal expected_changes, pirate.changes + end + test "changes is correct if override attribute reader" do pirate = Pirate.create!(catchphrase: "arrrr") def pirate.catchphrase -- cgit v1.2.3 From 5c8b3391a3d18ff3c3dd822d1531b074afa40675 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Apr 2017 18:17:10 +0900 Subject: Rename `association_query_handler.rb` to `association_query_value.rb` Since `AssociationQueryHandler` and `PolymorphicArrayHandler` has removed in #28715, only exists `AssociationQueryValue` and `PolymorphicArrayValue` in these files. --- .../active_record/relation/predicate_builder.rb | 17 ++++---- .../predicate_builder/association_query_handler.rb | 41 ------------------ .../predicate_builder/association_query_value.rb | 41 ++++++++++++++++++ .../predicate_builder/polymorphic_array_handler.rb | 49 ---------------------- .../predicate_builder/polymorphic_array_value.rb | 49 ++++++++++++++++++++++ 5 files changed, 99 insertions(+), 98 deletions(-) delete mode 100644 activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb create mode 100644 activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb delete mode 100644 activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb create mode 100644 activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 133b3d837c..183fe91c05 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -1,13 +1,14 @@ +require "active_record/relation/predicate_builder/array_handler" +require "active_record/relation/predicate_builder/base_handler" +require "active_record/relation/predicate_builder/basic_object_handler" +require "active_record/relation/predicate_builder/range_handler" +require "active_record/relation/predicate_builder/relation_handler" + +require "active_record/relation/predicate_builder/association_query_value" +require "active_record/relation/predicate_builder/polymorphic_array_value" + module ActiveRecord class PredicateBuilder # :nodoc: - require "active_record/relation/predicate_builder/array_handler" - require "active_record/relation/predicate_builder/association_query_handler" - require "active_record/relation/predicate_builder/base_handler" - require "active_record/relation/predicate_builder/basic_object_handler" - require "active_record/relation/predicate_builder/polymorphic_array_handler" - require "active_record/relation/predicate_builder/range_handler" - require "active_record/relation/predicate_builder/relation_handler" - delegate :resolve_column_aliases, to: :table def initialize(table) diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb deleted file mode 100644 index 2fe0f81cab..0000000000 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ /dev/null @@ -1,41 +0,0 @@ -module ActiveRecord - class PredicateBuilder - class AssociationQueryValue # :nodoc: - attr_reader :associated_table, :value - - def initialize(associated_table, value) - @associated_table = associated_table - @value = value - end - - def queries - [associated_table.association_foreign_key.to_s => ids] - end - - private - def ids - case value - when Relation - value.select_values.empty? ? value.select(primary_key) : value - when Array - value.map { |v| convert_to_id(v) } - else - convert_to_id(value) - end - end - - def primary_key - associated_table.association_primary_key - end - - def convert_to_id(value) - case value - when Base - value._read_attribute(primary_key) - else - value - end - end - end - end -end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb new file mode 100644 index 0000000000..2fe0f81cab --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb @@ -0,0 +1,41 @@ +module ActiveRecord + class PredicateBuilder + class AssociationQueryValue # :nodoc: + attr_reader :associated_table, :value + + def initialize(associated_table, value) + @associated_table = associated_table + @value = value + end + + def queries + [associated_table.association_foreign_key.to_s => ids] + end + + private + def ids + case value + when Relation + value.select_values.empty? ? value.select(primary_key) : value + when Array + value.map { |v| convert_to_id(v) } + else + convert_to_id(value) + end + end + + def primary_key + associated_table.association_primary_key + end + + def convert_to_id(value) + case value + when Base + value._read_attribute(primary_key) + else + value + end + end + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb deleted file mode 100644 index 9bb2f8c8dc..0000000000 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb +++ /dev/null @@ -1,49 +0,0 @@ -module ActiveRecord - class PredicateBuilder - class PolymorphicArrayValue # :nodoc: - attr_reader :associated_table, :values - - def initialize(associated_table, values) - @associated_table = associated_table - @values = values - end - - def queries - type_to_ids_mapping.map do |type, ids| - { - associated_table.association_foreign_type.to_s => type, - associated_table.association_foreign_key.to_s => ids.size > 1 ? ids : ids.first - } - end - end - - private - def type_to_ids_mapping - default_hash = Hash.new { |hsh, key| hsh[key] = [] } - values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } - end - - def primary_key(value) - associated_table.association_primary_key(base_class(value)) - end - - def base_class(value) - case value - when Base - value.class.base_class - when Relation - value.klass.base_class - end - end - - def convert_to_id(value) - case value - when Base - value._read_attribute(primary_key(value)) - when Relation - value.select(primary_key(value)) - end - end - end - end -end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb new file mode 100644 index 0000000000..9bb2f8c8dc --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb @@ -0,0 +1,49 @@ +module ActiveRecord + class PredicateBuilder + class PolymorphicArrayValue # :nodoc: + attr_reader :associated_table, :values + + def initialize(associated_table, values) + @associated_table = associated_table + @values = values + end + + def queries + type_to_ids_mapping.map do |type, ids| + { + associated_table.association_foreign_type.to_s => type, + associated_table.association_foreign_key.to_s => ids.size > 1 ? ids : ids.first + } + end + end + + private + def type_to_ids_mapping + default_hash = Hash.new { |hsh, key| hsh[key] = [] } + values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + end + + def primary_key(value) + associated_table.association_primary_key(base_class(value)) + end + + def base_class(value) + case value + when Base + value.class.base_class + when Relation + value.klass.base_class + end + end + + def convert_to_id(value) + case value + when Base + value._read_attribute(primary_key(value)) + when Relation + value.select(primary_key(value)) + end + end + end + end +end -- cgit v1.2.3 From c1a2cbf12821508b2db12d98fa327fd1ea7561bc Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Apr 2017 18:33:47 +0900 Subject: Early return in `PredicateBuilder::ArrayHandler` Partitioning to `values` and `nils` is unneeded before early return. --- .../lib/active_record/relation/predicate_builder/array_handler.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 54e9910598..1068e700e2 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -6,12 +6,11 @@ module ActiveRecord end def call(attribute, value) + return attribute.in([]) if value.empty? + return queries_predicates(value) if value.all? { |v| v.is_a?(Hash) } + values = value.map { |x| x.is_a?(Base) ? x.id : x } nils, values = values.partition(&:nil?) - - return attribute.in([]) if values.empty? && nils.empty? - return queries_predicates(values) if nils.empty? && values.all? { |v| v.is_a?(Hash) } - ranges, values = values.partition { |v| v.is_a?(Range) } values_predicate = -- cgit v1.2.3 From 1a92ae8318c3f5720907dec671d9ebb9221c0817 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 16 Apr 2017 07:45:33 +0900 Subject: Refactor `indexes` things in connection adapters * Use keyword arguments in `IndexDefinition` to ease to ignore unused options and to avoid to initialize incorrect empty value. * Place it in `SchemaStatements` for consistency. * And tiny tweaks. --- .../abstract/schema_definitions.rb | 27 +++++++++++++- .../connection_adapters/abstract_mysql_adapter.rb | 33 ----------------- .../connection_adapters/mysql/schema_statements.rb | 42 ++++++++++++++++++++++ .../postgresql/schema_statements.rb | 13 +++++-- .../sqlite3/schema_statements.rb | 35 ++++++++++++++++++ .../connection_adapters/sqlite3_adapter.rb | 31 ---------------- 6 files changed, 114 insertions(+), 67 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 4682afc188..7cafbb7e12 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -3,7 +3,32 @@ module ActiveRecord # Abstract representation of an index definition on a table. Instances of # this type are typically created and returned by methods in database # adapters. e.g. ActiveRecord::ConnectionAdapters::AbstractMysqlAdapter#indexes - IndexDefinition = Struct.new(:table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment) #:nodoc: + class IndexDefinition # :nodoc: + attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment + + def initialize( + table, name, + unique = false, + columns = [], + lengths: {}, + orders: {}, + where: nil, + type: nil, + using: nil, + comment: nil + ) + @table = table + @name = name + @unique = unique + @columns = columns + @lengths = lengths + @orders = orders + @where = where + @type = type + @using = using + @comment = comment + end + end # Abstract representation of a column definition. Instances of this type # are typically created by methods in TableDefinition, and added to the 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 f118e086bb..0866f58015 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -48,9 +48,6 @@ module ActiveRecord json: { name: "json" }, } - INDEX_TYPES = [:fulltext, :spatial] - INDEX_USINGS = [:btree, :hash] - class StatementPool < ConnectionAdapters::StatementPool private def dealloc(stmt) stmt[:stmt].close @@ -304,36 +301,6 @@ module ActiveRecord execute "TRUNCATE TABLE #{quote_table_name(table_name)}", name end - # Returns an array of indexes for the given table. - def indexes(table_name, name = nil) #:nodoc: - if name - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Passing name to #indexes is deprecated without replacement. - MSG - end - - indexes = [] - current_index = nil - execute_and_free("SHOW KEYS FROM #{quote_table_name(table_name)}", "SCHEMA") do |result| - each_hash(result) do |row| - if current_index != row[:Key_name] - next if row[:Key_name] == "PRIMARY" # skip the primary key - current_index = row[:Key_name] - - mysql_index_type = row[:Index_type].downcase.to_sym - index_type = INDEX_TYPES.include?(mysql_index_type) ? mysql_index_type : nil - index_using = INDEX_USINGS.include?(mysql_index_type) ? mysql_index_type : nil - indexes << IndexDefinition.new(row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, [], {}, nil, nil, index_type, index_using, row[:Index_comment].presence) - end - - indexes.last.columns << row[:Column_name] - indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] - end - end - - indexes - end - def table_comment(table_name) # :nodoc: scope = quoted_scope(table_name) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 9e2d0fb5e7..1e58513387 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -2,6 +2,48 @@ module ActiveRecord module ConnectionAdapters module MySQL module SchemaStatements # :nodoc: + # Returns an array of indexes for the given table. + def indexes(table_name, name = nil) + if name + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing name to #indexes is deprecated without replacement. + MSG + end + + indexes = [] + current_index = nil + execute_and_free("SHOW KEYS FROM #{quote_table_name(table_name)}", "SCHEMA") do |result| + each_hash(result) do |row| + if current_index != row[:Key_name] + next if row[:Key_name] == "PRIMARY" # skip the primary key + current_index = row[:Key_name] + + mysql_index_type = row[:Index_type].downcase.to_sym + case mysql_index_type + when :fulltext, :spatial + index_type = mysql_index_type + when :btree, :hash + index_using = mysql_index_type + end + + indexes << IndexDefinition.new( + row[:Table], + row[:Key_name], + row[:Non_unique].to_i == 0, + type: index_type, + using: index_using, + comment: row[:Index_comment].presence + ) + end + + indexes.last.columns << row[:Column_name] + indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + end + end + + indexes + end + private def schema_creation MySQL::SchemaCreation.new(self) 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 02a6da2f71..5b483ad4ab 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -140,8 +140,17 @@ module ActiveRecord ] end - IndexDefinition.new(table_name, index_name, unique, columns, [], orders, where, nil, using.to_sym, comment.presence) - end.compact + IndexDefinition.new( + table_name, + index_name, + unique, + columns, + orders: orders, + where: where, + using: using.to_sym, + comment: comment.presence + ) + end end def table_options(table_name) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index 8066a05c5e..e02491edb6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -2,6 +2,41 @@ module ActiveRecord module ConnectionAdapters module SQLite3 module SchemaStatements # :nodoc: + # Returns an array of indexes for the given table. + def indexes(table_name, name = nil) + if name + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing name to #indexes is deprecated without replacement. + MSG + end + + exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").map do |row| + index_sql = select_value(<<-SQL, "SCHEMA") + SELECT sql + FROM sqlite_master + WHERE name = #{quote(row['name'])} AND type = 'index' + UNION ALL + SELECT sql + FROM sqlite_temp_master + WHERE name = #{quote(row['name'])} AND type = 'index' + SQL + + /\sWHERE\s+(?.+)$/i =~ index_sql + + columns = exec_query("PRAGMA index_info(#{quote(row['name'])})", "SCHEMA").map do |col| + col["name"] + end + + IndexDefinition.new( + table_name, + row["name"], + row["unique"] != 0, + columns, + where: where + ) + end + end + private def schema_creation SQLite3::SchemaCreation.new(self) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index c54b88f7d1..e2c05ccc4e 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -259,37 +259,6 @@ module ActiveRecord # SCHEMA STATEMENTS ======================================== - # Returns an array of indexes for the given table. - def indexes(table_name, name = nil) #:nodoc: - if name - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Passing name to #indexes is deprecated without replacement. - MSG - end - - exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").map do |row| - sql = <<-SQL - SELECT sql - FROM sqlite_master - WHERE name=#{quote(row['name'])} AND type='index' - UNION ALL - SELECT sql - FROM sqlite_temp_master - WHERE name=#{quote(row['name'])} AND type='index' - SQL - index_sql = exec_query(sql).first["sql"] - match = /\sWHERE\s+(.+)$/i.match(index_sql) - where = match[1] if match - IndexDefinition.new( - table_name, - row["name"], - row["unique"] != 0, - exec_query("PRAGMA index_info('#{row['name']}')", "SCHEMA").map { |col| - col["name"] - }, nil, nil, where) - end - end - def primary_keys(table_name) # :nodoc: pks = table_structure(table_name).select { |f| f["pk"] > 0 } pks.sort_by { |f| f["pk"] }.map { |f| f["name"] } -- cgit v1.2.3 From 606830d27a32fab23c0964b4383807fcdfdd7eba Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 16 Apr 2017 05:17:14 +0900 Subject: Support Descending Indexes for MySQL MySQL 8.0.1 and higher supports descending indexes: `DESC` in an index definition is no longer ignored. See https://dev.mysql.com/doc/refman/8.0/en/descending-indexes.html. --- activerecord/CHANGELOG.md | 7 +++++++ .../active_record/connection_adapters/abstract_mysql_adapter.rb | 4 +--- .../active_record/connection_adapters/mysql/schema_statements.rb | 1 + activerecord/test/cases/schema_dumper_test.rb | 6 +++++- 4 files changed, 14 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ffe588b3c5..f74425b281 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Support Descending Indexes for MySQL. + + MySQL 8.0.1 and higher supports descending indexes: `DESC` in an index definition is no longer ignored. + See https://dev.mysql.com/doc/refman/8.0/en/descending-indexes.html. + + *Ryuta Kamizono* + * Fix inconsistency with changed attributes when overriding AR attribute reader. *bogdanvlviv* 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 0866f58015..71fa7b929c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -90,10 +90,8 @@ module ActiveRecord true end - # Technically MySQL allows to create indexes with the sort order syntax - # but at the moment (5.5) it doesn't yet implement them def supports_index_sort_order? - true + !mariadb? && version >= "8.0.1" end def supports_transaction_isolation? diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 1e58513387..571edffec7 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -38,6 +38,7 @@ module ActiveRecord indexes.last.columns << row[:Column_name] indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + indexes.last.orders.merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index fccba4738f..cb8d449ba9 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -182,7 +182,11 @@ class SchemaDumperTest < ActiveRecord::TestCase if current_adapter?(:PostgreSQLAdapter) assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition elsif current_adapter?(:Mysql2Adapter) - assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }', index_definition + if ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }, order: { rating: :desc }', index_definition + else + assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }', index_definition + end else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index"', index_definition end -- cgit v1.2.3 From c2dbdd814097031d437aca06f46032c8111d4880 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 17 Apr 2017 08:56:10 +0900 Subject: `cache_key` respects the limit in a relation even if a relation is not loaded `cache_key` includes the size of a relation. But if a relation is not loadded, the size is not respected even if a relation has a limit. It should be respected for consistency. --- .../lib/active_record/collection_cache_key.rb | 22 +++++++++++----- .../test/cases/collection_cache_key_test.rb | 30 ++++++++++++++++++++-- 2 files changed, 44 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/collection_cache_key.rb b/activerecord/lib/active_record/collection_cache_key.rb index 43784b70e3..1e1de1863a 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -7,17 +7,27 @@ module ActiveRecord if collection.loaded? size = collection.size if size > 0 - timestamp = collection.max_by(×tamp_column).public_send(timestamp_column) + timestamp = collection.max_by(×tamp_column)._read_attribute(timestamp_column) end else column_type = type_for_attribute(timestamp_column.to_s) column = "#{connection.quote_table_name(collection.table_name)}.#{connection.quote_column_name(timestamp_column)}" + select_values = "COUNT(*) AS #{connection.quote_column_name("size")}, MAX(%s) AS timestamp" - query = collection - .unscope(:select) - .select("COUNT(*) AS #{connection.quote_column_name("size")}", "MAX(#{column}) AS timestamp") - .unscope(:order) - result = connection.select_one(query) + if collection.limit_value || collection.offset_value + query = collection.spawn + query.select_values = [column] + subquery_alias = "subquery_for_cache_key" + subquery_column = "#{subquery_alias}.#{timestamp_column}" + subquery = query.arel.as(subquery_alias) + arel = Arel::SelectManager.new(query.engine).project(select_values % subquery_column).from(subquery) + else + query = collection.unscope(:order) + query.select_values = [select_values % column] + arel = query.arel + end + + result = connection.select_one(arel, nil, query.bound_attributes) if result.blank? size = 0 diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index 381a78a8e2..92620761e4 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -15,8 +15,34 @@ module ActiveRecord end test "cache_key for relation" do - developers = Developer.where(name: "David") - last_developer_timestamp = developers.order(updated_at: :desc).first.updated_at + developers = Developer.where(salary: 100000).order(updated_at: :desc) + last_developer_timestamp = developers.first.updated_at + + assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\Z/, developers.cache_key) + + /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\Z/ =~ developers.cache_key + + assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal developers.count.to_s, $2 + assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 + end + + test "cache_key for relation with limit" do + developers = Developer.where(salary: 100000).order(updated_at: :desc).limit(5) + last_developer_timestamp = developers.first.updated_at + + assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\Z/, developers.cache_key) + + /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\Z/ =~ developers.cache_key + + assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal developers.count.to_s, $2 + assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 + end + + test "cache_key for loaded relation" do + developers = Developer.where(salary: 100000).order(updated_at: :desc).limit(5).load + last_developer_timestamp = developers.first.updated_at assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\Z/, developers.cache_key) -- cgit v1.2.3 From a8ee3e8836fc5467d048a6334e94c198f086a737 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 12 Apr 2017 08:04:26 +0900 Subject: Don't fallback to utf8mb3 after MySQL 8.0.0 `internal_string_options_for_primary_key` is used for creating internal tables in newly apps. But it is no longer needed after MySQL 8.0.0. MySQL 5.7 has introduced `innodb_default_row_format` (default `DYNAMIC`) and has deprecated `innodb_large_prefix` and `innodb_file_format`. The purpose of the deprecated options was for compatibility with earlier versions of InnoDB. https://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix > innodb_large_prefix is deprecated and will be removed in a future release. innodb_large_prefix was introduced in MySQL 5.5 to disable large index key prefixes for compatibility with earlier versions of InnoDB that do not support large index key prefixes. https://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_file_format > The innodb_file_format option is deprecated and will be removed in a future release. The purpose of the innodb_file_format option was to allow users to downgrade to the built-in version of InnoDB in MySQL 5.1. Now that MySQL 5.1 has reached the end of its product lifecycle, downgrade support provided by this option is no longer necessary. The deprecated options has removed in MySQL 8.0.0. It is no longer needed to take care newly created internal tables as a legacy format after MySQL 8.0.0. Fixes #28730. --- .../connection_adapters/abstract_mysql_adapter.rb | 8 -------- .../connection_adapters/mysql/schema_statements.rb | 10 ++++++++++ .../test/cases/adapters/mysql2/schema_migrations_test.rb | 4 ++-- 3 files changed, 12 insertions(+), 10 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 71fa7b929c..c742799aab 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -64,14 +64,6 @@ module ActiveRecord end end - CHARSETS_OF_4BYTES_MAXLEN = ["utf8mb4", "utf16", "utf16le", "utf32"] - - def internal_string_options_for_primary_key # :nodoc: - super.tap { |options| - options[:collation] = collation.sub(/\A[^_]+/, "utf8") if CHARSETS_OF_4BYTES_MAXLEN.include?(charset) - } - end - def version #:nodoc: @version ||= Version.new(full_version.match(/^\d+\.\d+\.\d+/)[0]) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 571edffec7..f9e1e046ea 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -45,7 +45,17 @@ module ActiveRecord indexes end + def internal_string_options_for_primary_key + super.tap do |options| + if CHARSETS_OF_4BYTES_MAXLEN.include?(charset) && (mariadb? || version < "8.0.0") + options[:collation] = collation.sub(/\A[^_]+/, "utf8") + end + end + end + private + CHARSETS_OF_4BYTES_MAXLEN = ["utf8mb4", "utf16", "utf16le", "utf32"] + def schema_creation MySQL::SchemaCreation.new(self) end diff --git a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb index 605baa9905..251a50e41e 100644 --- a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb +++ b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb @@ -18,7 +18,7 @@ class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase ActiveRecord::SchemaMigration.create_table - assert connection.column_exists?(table_name, :version, :string, collation: "utf8_general_ci") + assert connection.column_exists?(table_name, :version, :string) end end @@ -29,7 +29,7 @@ class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase ActiveRecord::InternalMetadata.create_table - assert connection.column_exists?(table_name, :key, :string, collation: "utf8_general_ci") + assert connection.column_exists?(table_name, :key, :string) end end -- cgit v1.2.3 From ee8beaa41bbe6ed47d40be45ce37abb3fb1b79be Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 17 Apr 2017 16:07:08 +0900 Subject: Fix the doc on the `IndexDefinition` [ci skip] Since 1a92ae83 all `indexes` methods are under the `SchemaStatements`. --- .../active_record/connection_adapters/abstract/schema_definitions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 7cafbb7e12..46d7f84efd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -2,7 +2,7 @@ module ActiveRecord module ConnectionAdapters #:nodoc: # Abstract representation of an index definition on a table. Instances of # this type are typically created and returned by methods in database - # adapters. e.g. ActiveRecord::ConnectionAdapters::AbstractMysqlAdapter#indexes + # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes class IndexDefinition # :nodoc: attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment -- cgit v1.2.3 From 791f20167bf22ca43853d6cbbf29367d9b7ac96d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 19 Apr 2017 12:08:38 +0900 Subject: Use `quoted_scope` rather than `@config[:database]` to respect current database Related #28399. --- .../lib/active_record/connection_adapters/mysql/schema_dumper.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index 3e0afd9761..b66db0f6fb 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -59,9 +59,10 @@ module ActiveRecord $~[:expression].inspect end else + scope = quoted_scope(column.table_name) sql = "SELECT generation_expression FROM information_schema.columns" \ - " WHERE table_schema = #{quote(@config[:database])}" \ - " AND table_name = #{quote(column.table_name)}" \ + " WHERE table_schema = #{scope[:schema]}" \ + " AND table_name = #{scope[:name]}" \ " AND column_name = #{quote(column.name)}" select_value(sql, "SCHEMA").inspect end -- cgit v1.2.3 From 7695f35baa0f45c5742de03a9b26595b2bc49008 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 19 Apr 2017 12:55:24 +0900 Subject: Fix `extract_expression_for_virtual_column` for MariaDB I'm not sure why `Mysql2VirtualColumnTest#test_schema_dumping` passed previously. But now the test not pass at least in MariaDB 10.1.9. I fixed the regexp to respect `COLLATE`. ``` % ARCONN=mysql2 be ruby -w -Itest test/cases/adapters/mysql2/virtual_column_test.rb -n test_schema_dumping Using mysql2 Run options: -n test_schema_dumping --seed 7131 F Finished in 0.466304s, 2.1445 runs/s, 4.2890 assertions/s. 1) Failure: Mysql2VirtualColumnTest#test_schema_dumping [test/cases/adapters/mysql2/virtual_column_test.rb:55]: Expected /t\.virtual\s+"upper_name",\s+type: :string,\s+as: "UPPER\(`name`\)"$/i to match "# This file is auto-generated from the current state of the database. Instead\n# of editing this file, please use the migrations feature of Active Record to\n# incrementally modify your database, and then regenerate this schema definition.\n#\n# Note that this schema.rb definition is the authoritative source for your\n# database schema. If you need to create the application database on another\n# system, you should be using db:schema:load, not running all the migrations\n# from scratch. The latter is a flawed and unsustainable approach (the more migrations\n# you'll amass, the slower it'll run and the greater likelihood for issues).\n#\n# It's strongly recommended that you check this file into your version control system.\n\nActiveRecord::Schema.define(version: 0) do\n\n create_table \"virtual_columns\", force: :cascade, options: \"ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci\" do |t|\n t.string \"name\"\n t.virtual \"upper_name\", type: :string, as: \n t.virtual \"name_length\", type: :integer, as: \"LENGTH(`name`)\", stored: true\n end\n\nend\n". 1 runs, 2 assertions, 1 failures, 0 errors, 0 skips ``` ``` > select @@version; +--------------------+ | @@version | +--------------------+ | 10.1.9-MariaDB-log | +--------------------+ 1 row in set (0.00 sec) ``` --- .../lib/active_record/connection_adapters/mysql/schema_dumper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index b66db0f6fb..e2ba0ba1a0 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -55,7 +55,7 @@ module ActiveRecord def extract_expression_for_virtual_column(column) if mariadb? create_table_info = create_table_info(column.table_name) - if %r/#{quote_column_name(column.name)} #{Regexp.quote(column.sql_type)} AS \((?.+?)\) #{column.extra}/m =~ create_table_info + if %r/#{quote_column_name(column.name)} #{Regexp.quote(column.sql_type)}(?: COLLATE \w+)? AS \((?.+?)\) #{column.extra}/ =~ create_table_info $~[:expression].inspect end else -- cgit v1.2.3 From b77d2aa0c336492ba33cbfade4964ba0eda3ef84 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 25 Feb 2017 17:49:32 +0200 Subject: Fix `bin/rails db:forward` first migration --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/migration.rb | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f74425b281..d8f93280cd 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix `bin/rails db:forward` first migration. + + *bogdanvlviv* + * Support Descending Indexes for MySQL. MySQL 8.0.1 and higher supports descending indexes: `DESC` in an index definition is no longer ignored. diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 4e1df1432c..43b6a746e0 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1104,7 +1104,13 @@ module ActiveRecord def move(direction, migrations_paths, steps) migrator = new(direction, migrations(migrations_paths)) - start_index = migrator.migrations.index(migrator.current_migration) + + start_index = + if current_version == 0 + 0 + else + migrator.migrations.index(migrator.current_migration) + end if start_index finish = migrator.migrations[start_index + steps] -- cgit v1.2.3 From bb9d6eb094f29bb94ef1f26aa44f145f17b973fe Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 25 Feb 2017 19:32:50 +0200 Subject: Add additional raise UnknownMigrationVersionError Raise error on the movement of migrations when the current migration does not exist. --- activerecord/CHANGELOG.md | 5 +++++ activerecord/lib/active_record/migration.rb | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d8f93280cd..f0cfd0469e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Raise error `UnknownMigrationVersionError` on the movement of migrations + when the current migration does not exist. + + *bogdanvlviv* + * Fix `bin/rails db:forward` first migration. *bogdanvlviv* diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 43b6a746e0..51c82f4ced 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1105,6 +1105,10 @@ module ActiveRecord def move(direction, migrations_paths, steps) migrator = new(direction, migrations(migrations_paths)) + if current_version != 0 && !migrator.current_migration + raise UnknownMigrationVersionError.new(current_version) + end + start_index = if current_version == 0 0 @@ -1112,11 +1116,9 @@ module ActiveRecord migrator.migrations.index(migrator.current_migration) end - if start_index - finish = migrator.migrations[start_index + steps] - version = finish ? finish.version : 0 - send(direction, migrations_paths, version) - end + finish = migrator.migrations[start_index + steps] + version = finish ? finish.version : 0 + send(direction, migrations_paths, version) end end -- cgit v1.2.3 From 2d699e24ff10158c0831cf6b7f5e5b12ac41903a Mon Sep 17 00:00:00 2001 From: Rune Schjellerup Philosof Date: Thu, 6 Apr 2017 14:05:49 +0200 Subject: Fix quoting in db:create grant all statement. The database name used in the test would have actually shown this if it had tried to execute on a real Mysql instead of being stubbed out (dashes in database names needs quotes). --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/tasks/mysql_database_tasks.rb | 2 +- activerecord/test/cases/tasks/mysql_rake_test.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 30d580b9e3..d4a68a39f9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1 +1,5 @@ +* Quote database name in db:create grant statement (when database_user does not have access to create the database). + + *Rune Philosof* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 920830b9cf..c05f0a8fbb 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -104,7 +104,7 @@ module ActiveRecord def grant_statement <<-SQL -GRANT ALL PRIVILEGES ON #{configuration['database']}.* +GRANT ALL PRIVILEGES ON `#{configuration['database']}`.* TO '#{configuration['username']}'@'localhost' IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; SQL diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index f30e0958c3..b85d303a91 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -167,7 +167,7 @@ if current_adapter?(:Mysql2Adapter) def assert_permissions_granted_for(db_user) db_name = @configuration["database"] db_password = @configuration["password"] - @connection.expects(:execute).with("GRANT ALL PRIVILEGES ON #{db_name}.* TO '#{db_user}'@'localhost' IDENTIFIED BY '#{db_password}' WITH GRANT OPTION;") + @connection.expects(:execute).with("GRANT ALL PRIVILEGES ON `#{db_name}`.* TO '#{db_user}'@'localhost' IDENTIFIED BY '#{db_password}' WITH GRANT OPTION;") end end -- cgit v1.2.3 From c0038f7c362fa0c92fc9e1ea3bdb2706f42386c6 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 2 Apr 2017 08:24:23 +0900 Subject: Prevent double firing the before save callback of new object when the parent association saved in the callback Related #18155, #26661, 268a5bb, #27434, #27442, and #28599. Originally #18155 was introduced for preventing double insertion caused by the after save callback. But it was caused the before save issue (#26661). 268a5bb fixed #26661, but it was caused the performance regression (#27434). #27442 added new record to `target` before calling callbacks for fixing #27434. But it was caused double firing before save callback (#28599). We cannot add new object to `target` before saving the object. This is improving #18155 to only track callbacks after `save`. Fixes #28599. --- .../associations/collection_association.rb | 67 +++++++++++----------- .../associations/has_many_association.rb | 7 +-- .../associations/has_many_through_association.rb | 6 +- activerecord/lib/active_record/persistence.rb | 26 +++++++-- .../associations/has_many_associations_test.rb | 18 +++++- 5 files changed, 70 insertions(+), 54 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 77282e6463..62c944fce3 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -280,35 +280,6 @@ module ActiveRecord replace_on_target(record, index, skip_callbacks, &block) end - def replace_on_target(record, index, skip_callbacks) - callback(:before_add, record) unless skip_callbacks - - begin - if index - record_was = target[index] - target[index] = record - else - target << record - end - - set_inverse_instance(record) - - yield(record) if block_given? - rescue - if index - target[index] = record_was - else - target.delete(record) - end - - raise - end - - callback(:after_add, record) unless skip_callbacks - - record - end - def scope scope = super scope.none! if null_scope? @@ -385,15 +356,19 @@ module ActiveRecord transaction do add_to_target(build_record(attributes)) do |record| yield(record) if block_given? - insert_record(record, true, raise) + insert_record(record, true, raise) { @_was_loaded = loaded? } end end end end # Do the relevant stuff to insert the given record into the association collection. - def insert_record(record, validate = true, raise = false) - raise NotImplementedError + def insert_record(record, validate = true, raise = false, &block) + if raise + record.save!(validate: validate, &block) + else + record.save(validate: validate, &block) + end end def create_scope @@ -448,19 +423,41 @@ module ActiveRecord end end - def concat_records(records, should_raise = false) + def concat_records(records, raise = false) result = true records.each do |record| raise_on_type_mismatch!(record) - add_to_target(record) do |rec| - result &&= insert_record(rec, true, should_raise) unless owner.new_record? + add_to_target(record) do + result &&= insert_record(record, true, raise) { @_was_loaded = loaded? } unless owner.new_record? end end result && records end + def replace_on_target(record, index, skip_callbacks) + callback(:before_add, record) unless skip_callbacks + + set_inverse_instance(record) + + @_was_loaded = true + + yield(record) if block_given? + + if index + target[index] = record + elsif @_was_loaded || !loaded? + target << record + end + + callback(:after_add, record) unless skip_callbacks + + record + ensure + @_was_loaded = nil + end + def callback(method, record) callbacks_for(method).each do |callback| callback.call(method, owner, record) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index b413eb3f9c..10ca0e47ff 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -31,12 +31,7 @@ module ActiveRecord def insert_record(record, validate = true, raise = false) set_owner_attributes(record) - - if raise - record.save!(validate: validate) - else - record.save(validate: validate) - end + super end def empty? 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 c4a7fe4432..53ffb3b68d 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -39,11 +39,7 @@ module ActiveRecord ensure_not_nested if record.new_record? || record.has_changes_to_save? - if raise - record.save!(validate: validate) - else - return unless record.save(validate: validate) - end + return unless super end save_through_record(record) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 7ceb7d1a55..f652c7c3a1 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -100,6 +100,10 @@ module ActiveRecord !(@new_record || @destroyed) end + ## + # :call-seq: + # save(*args) + # # Saves the model. # # If the model is new, a record gets created in the database, otherwise @@ -121,12 +125,16 @@ module ActiveRecord # # Attributes marked as readonly are silently ignored if the record is # being updated. - def save(*args) - create_or_update(*args) + def save(*args, &block) + create_or_update(*args, &block) rescue ActiveRecord::RecordInvalid false end + ## + # :call-seq: + # save!(*args) + # # Saves the model. # # If the model is new, a record gets created in the database, otherwise @@ -150,8 +158,8 @@ module ActiveRecord # being updated. # # Unless an error is raised, returns true. - def save!(*args) - create_or_update(*args) || raise(RecordNotSaved.new("Failed to save the record", self)) + def save!(*args, &block) + create_or_update(*args, &block) || raise(RecordNotSaved.new("Failed to save the record", self)) end # Deletes the record in the database and freezes this instance to @@ -550,9 +558,9 @@ module ActiveRecord self.class.unscoped.where(self.class.primary_key => id) end - def create_or_update(*args) + def create_or_update(*args, &block) _raise_readonly_record_error if readonly? - result = new_record? ? _create_record : _update_record(*args) + result = new_record? ? _create_record(&block) : _update_record(*args, &block) result != false end @@ -567,6 +575,9 @@ module ActiveRecord rows_affected = self.class.unscoped._update_record attributes_values, id, id_in_database @_trigger_update_callback = rows_affected > 0 end + + yield(self) if block_given? + rows_affected end @@ -579,6 +590,9 @@ module ActiveRecord self.id ||= new_id if self.class.primary_key @new_record = false + + yield(self) if block_given? + id end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index e2f044c139..25133fc580 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2447,15 +2447,29 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [first_bulb, second_bulb], car.bulbs end - test "double insertion of new object to association when same association used in the after create callback of a new object" do + test "prevent double insertion of new object when the parent association loaded in the after save callback" do reset_callbacks(:save, Bulb) do Bulb.after_save { |record| record.car.bulbs.load } + car = Car.create! car.bulbs << Bulb.new + assert_equal 1, car.bulbs.size end end + test "prevent double firing the before save callback of new object when the parent association saved in the callback" do + reset_callbacks(:save, Bulb) do + count = 0 + Bulb.before_save { |record| record.car.save && count += 1 } + + car = Car.create! + car.bulbs.create! + + assert_equal 1, count + end + end + class AuthorWithErrorDestroyingAssociation < ActiveRecord::Base self.table_name = "authors" has_many :posts_with_error_destroying, @@ -2496,7 +2510,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_loading_association_in_validate_callback_doesnt_affect_persistence reset_callbacks(:validation, Bulb) do - Bulb.after_validation { |m| m.car.bulbs.load } + Bulb.after_validation { |record| record.car.bulbs.load } car = Car.create!(name: "Car") bulb = car.bulbs.create! -- cgit v1.2.3 From b17fa0c353a32c08558892f722f509f8699dba35 Mon Sep 17 00:00:00 2001 From: Isaac Betesh Date: Wed, 29 Mar 2017 11:05:38 -0400 Subject: Don't attempt to create a new record that was already created. Fixes #24032 --- .../lib/active_record/autosave_association.rb | 5 +++++ .../has_and_belongs_to_many_associations_test.rb | 20 ++++++++++++++++++++ activerecord/test/schema/schema.rb | 7 +++++++ 3 files changed, 32 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6bccbc06cd..607c54e481 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -181,6 +181,7 @@ module ActiveRecord if reflection.collection? before_save :before_save_collection_association + after_save :after_save_collection_association define_non_cyclic_method(save_method) { save_collection_association(reflection) } # Doesn't use after_save as that would save associations added in after_create/after_update twice @@ -371,6 +372,10 @@ module ActiveRecord true end + def after_save_collection_association + @new_record_before_save = false + end + # Saves any new associated records, or all loaded autosave associations if # :autosave is enabled on the association. # diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index d6b595d7e7..8060790594 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -111,6 +111,21 @@ class ProjectUnscopingDavidDefaultScope < ActiveRecord::Base association_foreign_key: "developer_id" end +class Kitchen < ActiveRecord::Base + has_one :sink +end + +class Sink < ActiveRecord::Base + has_and_belongs_to_many :sources, join_table: :edges + belongs_to :kitchen + accepts_nested_attributes_for :kitchen +end + +class Source < ActiveRecord::Base + self.table_name = "men" + has_and_belongs_to_many :sinks, join_table: :edges +end + class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects, :parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings, :computers @@ -1021,4 +1036,9 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase ActiveRecord::Base.partial_writes = original_partial_writes end end + + def test_has_and_belongs_to_many_with_belongs_to + sink = Sink.create! kitchen: Kitchen.new, sources: [Source.new] + assert_equal 1, sink.sources.count + end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 08bef08abc..50f1d9bfe7 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -413,6 +413,9 @@ ActiveRecord::Schema.define do t.string :name end + create_table :kitchens, force: true do |t| + end + create_table :legacy_things, force: true do |t| t.integer :tps_report_number t.integer :version, null: false, default: 0 @@ -783,6 +786,10 @@ ActiveRecord::Schema.define do t.belongs_to :ship end + create_table :sinks, force: true do |t| + t.references :kitchen + end + create_table :shop_accounts, force: true do |t| t.references :customer t.references :customer_carrier -- cgit v1.2.3 From e5e3daf7e2cafbfd6d41eae29752d0a0e4429052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Fankh=C3=A4nel?= Date: Tue, 25 Apr 2017 16:52:44 +0200 Subject: Fix typos [ci skip] 'lookup' is the noun. 'to look up' is the verb. Looked it up just to be sure. cf. https://en.wiktionary.org/wiki/lookup https://en.wiktionary.org/wiki/look_up --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 53dbbd8c21..177b253645 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -857,9 +857,9 @@ module ActiveRecord # All Active Record models use this handler to determine the connection pool that they # should use. # - # The ConnectionHandler class is not coupled with the Active models, as it has no knowlodge + # The ConnectionHandler class is not coupled with the Active models, as it has no knowledge # about the model. The model needs to pass a specification name to the handler, - # in order to lookup the correct connection pool. + # in order to look up the correct connection pool. class ConnectionHandler def initialize # These caches are keyed by spec.name (ConnectionSpecification#name). -- cgit v1.2.3