From bbcd707aefe5da137137a8deb13908ec9a7db77d Mon Sep 17 00:00:00 2001 From: Vishal Telangre Date: Wed, 8 May 2019 18:11:43 +0530 Subject: Fix: ActiveRecord::RecordInvalid is not raised when an associated record fails to #save! due to uniqueness validation failure Add tests Fix tests failing due to introduction of uniquness rule added to Book model --- .../test/cases/autosave_association_test.rb | 41 ++++++++++++++++++++++ activerecord/test/models/author.rb | 1 + activerecord/test/models/book.rb | 6 ++++ 3 files changed, 48 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 1a0732c14b..45bb58eb06 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require "models/author" +require "models/book" require "models/bird" require "models/post" require "models/comment" @@ -1685,6 +1686,10 @@ class TestAutosaveAssociationValidationsOnAHasManyAssociation < ActiveRecord::Te super @pirate = Pirate.create(catchphrase: "Don' botharrr talkin' like one, savvy?") @pirate.birds.create(name: "cookoo") + + @author = Author.new(name: "DHH") + @author.published_books.build(name: "Rework", isbn: "1234") + @author.published_books.build(name: "Remote", isbn: "1234") end test "should automatically validate associations" do @@ -1693,6 +1698,42 @@ class TestAutosaveAssociationValidationsOnAHasManyAssociation < ActiveRecord::Te assert_not_predicate @pirate, :valid? end + + test "rollbacks whole transaction and raises ActiveRecord::RecordInvalid when associations fail to #save! due to uniqueness validation failure" do + author_count_before_save = Author.count + book_count_before_save = Book.count + + assert_no_difference "Author.count" do + assert_no_difference "Book.count" do + exception = assert_raises(ActiveRecord::RecordInvalid) do + @author.save! + end + + assert_equal("Validation failed: Published books is invalid", exception.message) + end + end + + assert_equal(author_count_before_save, Author.count) + assert_equal(book_count_before_save, Book.count) + end + + test "rollbacks whole transaction when associations fail to #save due to uniqueness validation failure" do + author_count_before_save = Author.count + book_count_before_save = Book.count + + assert_no_difference "Author.count" do + assert_no_difference "Book.count" do + assert_nothing_raised do + result = @author.save + + assert_not(result) + end + end + end + + assert_equal(author_count_before_save, Author.count) + assert_equal(book_count_before_save, Book.count) + end end class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index b52b643ad7..da7e4139b1 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -116,6 +116,7 @@ class Author < ActiveRecord::Base has_many :tags_with_primary_key, through: :posts has_many :books + has_many :published_books, class_name: "PublishedBook" has_many :unpublished_books, -> { where(status: [:proposed, :written]) }, class_name: "Book" has_many :subscriptions, through: :books has_many :subscribers, -> { order("subscribers.nick") }, through: :subscriptions diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb index afdda1a81e..43b82e6047 100644 --- a/activerecord/test/models/book.rb +++ b/activerecord/test/models/book.rb @@ -24,3 +24,9 @@ class Book < ActiveRecord::Base "do publish work..." end end + +class PublishedBook < ActiveRecord::Base + self.table_name = "books" + + validates_uniqueness_of :isbn +end -- cgit v1.2.3 From 17acb771d815f030ac1cf36b1af4050f02816c39 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 8 Apr 2019 13:10:15 +0200 Subject: Deduplicate various Active Record schema cache structures Real world database schemas contain a lot of duplicated data. Some column names like `id`, `created_at` etc can easily be repeated hundreds of times. Same for SqlTypeMetada, most database will contain only a limited number of possible combinations. This result in a lot of wasted memory. The idea here is to make these data sctructures immutable, use a registry to substitute similar instances with pre-existing ones. --- .../active_record/connection_adapters/fake_adapter.rb | 3 ++- activerecord/test/cases/base_test.rb | 9 ++++++--- activerecord/test/cases/json_serialization_test.rb | 2 +- activerecord/test/models/contact.rb | 18 +++++++++--------- 4 files changed, 18 insertions(+), 14 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/active_record/connection_adapters/fake_adapter.rb b/activerecord/test/active_record/connection_adapters/fake_adapter.rb index f977b2997b..f1f457aedd 100644 --- a/activerecord/test/active_record/connection_adapters/fake_adapter.rb +++ b/activerecord/test/active_record/connection_adapters/fake_adapter.rb @@ -32,7 +32,8 @@ module ActiveRecord name.to_s, options[:default], fetch_type_metadata(sql_type), - options[:null]) + options[:null], + ) end def columns(table_name) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index ddafa468ed..1c83100a75 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1141,11 +1141,14 @@ class BasicsTest < ActiveRecord::TestCase def test_clear_cache! # preheat cache c1 = Post.connection.schema_cache.columns("posts") + assert_not_equal 0, Post.connection.schema_cache.size + ActiveRecord::Base.clear_cache! + assert_equal 0, Post.connection.schema_cache.size + c2 = Post.connection.schema_cache.columns("posts") - c1.each_with_index do |v, i| - assert_not_same v, c2[i] - end + assert_not_equal 0, Post.connection.schema_cache.size + assert_equal c1, c2 end diff --git a/activerecord/test/cases/json_serialization_test.rb b/activerecord/test/cases/json_serialization_test.rb index 82cf281cff..8f9b1e2ec1 100644 --- a/activerecord/test/cases/json_serialization_test.rb +++ b/activerecord/test/cases/json_serialization_test.rb @@ -24,7 +24,7 @@ class JsonSerializationTest < ActiveRecord::TestCase include JsonSerializationHelpers class NamespacedContact < Contact - column :name, :string + column :name, "string" end def setup diff --git a/activerecord/test/models/contact.rb b/activerecord/test/models/contact.rb index 6e02ff199b..d5f6f00691 100644 --- a/activerecord/test/models/contact.rb +++ b/activerecord/test/models/contact.rb @@ -10,14 +10,14 @@ module ContactFakeColumns table_name => "id" } - column :id, :integer - column :name, :string - column :age, :integer - column :avatar, :binary - column :created_at, :datetime - column :awesome, :boolean - column :preferences, :string - column :alternative_id, :integer + column :id, "integer" + column :name, "string" + column :age, "integer" + column :avatar, "binary" + column :created_at, "datetime" + column :awesome, "boolean" + column :preferences, "string" + column :alternative_id, "integer" serialize :preferences @@ -37,7 +37,7 @@ end class ContactSti < ActiveRecord::Base extend ContactFakeColumns - column :type, :string + column :type, "string" def type; "ContactSti" end end -- cgit v1.2.3 From 2c96b046ec50e496ccdb6a196efcbbba5818a79c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 19 Jun 2019 23:18:22 +0900 Subject: Add test cases to ensure deterministic order for ordinal methods Before 1340498d2, `order` with no-op value (e.g. `nil`, `""`) had broken the contract of ordinal methods, which returns a result deterministic ordered. --- activerecord/test/cases/finder_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index ca114d468e..3aa610f86b 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -517,6 +517,7 @@ class FinderTest < ActiveRecord::TestCase expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.first assert_equal expected, Topic.limit(5).first + assert_equal expected, Topic.order(nil).first end def test_model_class_responds_to_first_bang @@ -540,6 +541,7 @@ class FinderTest < ActiveRecord::TestCase expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.second assert_equal expected, Topic.limit(5).second + assert_equal expected, Topic.order(nil).second end def test_model_class_responds_to_second_bang @@ -563,6 +565,7 @@ class FinderTest < ActiveRecord::TestCase expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.third assert_equal expected, Topic.limit(5).third + assert_equal expected, Topic.order(nil).third end def test_model_class_responds_to_third_bang @@ -586,6 +589,7 @@ class FinderTest < ActiveRecord::TestCase expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.fourth assert_equal expected, Topic.limit(5).fourth + assert_equal expected, Topic.order(nil).fourth end def test_model_class_responds_to_fourth_bang @@ -609,6 +613,7 @@ class FinderTest < ActiveRecord::TestCase expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.fifth assert_equal expected, Topic.limit(5).fifth + assert_equal expected, Topic.order(nil).fifth end def test_model_class_responds_to_fifth_bang @@ -777,6 +782,7 @@ class FinderTest < ActiveRecord::TestCase assert_equal expected, clients.first(2) assert_equal expected, clients.limit(5).first(2) + assert_equal expected, clients.order(nil).first(2) end def test_implicit_order_column_is_configurable -- cgit v1.2.3 From fd3532204c7302ec80a245c72852a11288ec38b5 Mon Sep 17 00:00:00 2001 From: Guilherme Mansur Date: Tue, 14 May 2019 15:13:29 -0400 Subject: Better error message for calling columns_hash When a record does not have a table name, as in the case for a record with `self.abstract_class = true` and no `self.table_name` set the error message raises a cryptic: "ActiveRecord::StatementInvalid: Could not find table ''" this patch now raises a new `TableNotSpecified Error` Fixes: #36274 Co-Authored-By: Eugene Kenny --- activerecord/test/cases/base_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 983a46a2d0..1324bdf9b8 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1415,6 +1415,14 @@ class BasicsTest < ActiveRecord::TestCase assert_not_includes SymbolIgnoredDeveloper.columns_hash.keys, "first_name" end + test ".columns_hash raises an error if the record has an empty table name" do + expected_message = "FirstAbstractClass has no table configured. Set one with FirstAbstractClass.table_name=" + exception = assert_raises(ActiveRecord::TableNotSpecified) do + FirstAbstractClass.columns_hash + end + assert_equal expected_message, exception.message + end + test "ignored columns have no attribute methods" do assert_not_respond_to Developer.new, :first_name assert_not_respond_to Developer.new, :first_name= -- cgit v1.2.3 From 4feeee2abeaf61eed2f9ee8463d3ac9661ec8cc6 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 20 Jun 2019 14:00:42 +0200 Subject: Revert schema dumper to use strings rather than integers I think we should change this, but not in 6-0-stable since that's already in RC and I was trying to only make changes that won't require any app changes. This reverts a portion of https://github.com/rails/rails/pull/36439 that made all schema migration version numbers get dumped as an integer. While it doesn't _really_ matter it did change behavior. We should bring this back in 6.1 with a deprecation. --- activerecord/test/cases/schema_dumper_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 49e9be9565..bb7184c5fc 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -33,6 +33,7 @@ class SchemaDumperTest < ActiveRecord::TestCase schema_info = ActiveRecord::Base.connection.dump_schema_information assert_match(/20100201010101.*20100301010101/m, schema_info) + assert_includes schema_info, "20100101010101" ensure ActiveRecord::SchemaMigration.delete_all end -- cgit v1.2.3 From 688da62a4c43438374fa58b3aa028082a15b5e9e Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Thu, 20 Jun 2019 14:12:58 +0000 Subject: Address test_statement_cache_with_in_clause failure due to nondeterministic sort order This failure is occasional, does not always reproduce. ```ruby $ cd activerecord $ bundle exec rake test_postgresql ... snip ... ....F Failure: ActiveRecord::BindParameterTest#test_statement_cache_with_in_clause [/home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:97]: Expected: [1, 3] Actual: [3, 1] rails test home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:93 ``` --- activerecord/test/cases/bind_parameter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 85685d1d00..720446b39d 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -93,7 +93,7 @@ if ActiveRecord::Base.connection.prepared_statements def test_statement_cache_with_in_clause @connection.clear_cache! - topics = Topic.where(id: [1, 3]) + topics = Topic.where(id: [1, 3]).order(:id) assert_equal [1, 3], topics.map(&:id) assert_not_includes statement_cache, to_sql_key(topics.arel) end -- cgit v1.2.3 From f6db8b8d8286730757570a8b0f92e6b6be779fa8 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 26 Jun 2019 08:56:00 +0900 Subject: `length(title)` is a safe SQL string since #36448 --- activerecord/test/cases/relations_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 5df1e3ccf9..1a20fe5dc2 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -308,9 +308,9 @@ class RelationTest < ActiveRecord::TestCase end def test_reverse_order_with_function_other_predicates - topics = Topic.order(Arel.sql("author_name, length(title), id")).reverse_order + topics = Topic.order("author_name, length(title), id").reverse_order assert_equal topics(:second).title, topics.first.title - topics = Topic.order(Arel.sql("length(author_name), id, length(title)")).reverse_order + topics = Topic.order("length(author_name), id, length(title)").reverse_order assert_equal topics(:fifth).title, topics.first.title end -- cgit v1.2.3 From 7e08b6d2b2b67edbd0314b3a15c9ed5633f1c753 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 28 Jun 2019 00:26:22 +0900 Subject: Address to "DEPRECATION WARNING: Uniqueness validator will no longer enforce case sensitive comparison in Rails 6.1" Caused by #36210. --- activerecord/test/schema/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/test') diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index eed18a7b89..b6c0ae0de2 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -115,7 +115,7 @@ ActiveRecord::Schema.define do t.column :font_size, :integer, **default_zero t.column :difficulty, :integer, **default_zero t.column :cover, :string, default: "hard" - t.string :isbn + t.string :isbn, **case_sensitive_options t.datetime :published_on t.index [:author_id, :name], unique: true t.index :isbn, where: "published_on IS NOT NULL", unique: true -- cgit v1.2.3 From f2ad69fe7a605b01bb7c37eeac6a9b4e7deb488e Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 27 Jun 2019 12:56:30 -0400 Subject: Fix broken url configs This PR is to fix #36559 but I also found other issues that haven't been reported. The check for `(config.size == 1 && config.values.all? { |v| v.is_a? String })` was naive. The only reason this passed was because we had tests that had single hash size configs, but that doesn't mean we don't want to create a hash config in other cases. So this now checks for `config["database"] || config["adapter"] || ENV["DATABASE_URL"]`. In the end for url configs we still get a UrlConfig but we need to pass through the HashConfig to create the right kind of UrlConfig. The UrlConfig's are really complex and I don't necessarily understand everything that's needed in order to act the same as Rails 5.2. I edited the connection handler test to demonstrate how the previous implementation was broken when checking config size. Now old and new tests pass so I think this is closer to 5.2. Fixes #36559 --- .../connection_adapters/connection_handler_test.rb | 2 +- .../merge_and_resolve_default_url_config_test.rb | 31 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 27589966af..843242a897 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -29,7 +29,7 @@ module ActiveRecord def test_establish_connection_uses_spec_name old_config = ActiveRecord::Base.configurations - config = { "readonly" => { "adapter" => "sqlite3" } } + config = { "readonly" => { "adapter" => "sqlite3", "pool" => "5" } } ActiveRecord::Base.configurations = config resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(ActiveRecord::Base.configurations) spec = resolver.spec(:readonly) diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 515bf5df06..c0a9f8f9ca 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -273,6 +273,37 @@ module ActiveRecord } assert_equal expected, actual end + + def test_merge_no_conflicts_with_database_url_and_adapter + ENV["DATABASE_URL"] = "postgres://localhost/foo" + + config = { "default_env" => { "adapter" => "postgresql", "pool" => "5" } } + actual = resolve_config(config) + expected = { "default_env" => + { "adapter" => "postgresql", + "database" => "foo", + "host" => "localhost", + "pool" => "5" + } + } + assert_equal expected, actual + end + + def test_merge_no_conflicts_with_database_url_and_numeric_pool + ENV["DATABASE_URL"] = "postgres://localhost/foo" + + config = { "default_env" => { "pool" => 5 } } + actual = resolve_config(config) + expected = { "default_env" => + { "adapter" => "postgresql", + "database" => "foo", + "host" => "localhost", + "pool" => 5 + } + } + + assert_equal expected, actual + end end end end -- cgit v1.2.3 From 52729fb5f24ff96c812327ff045b9a1921d37a7c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 Jul 2019 12:07:31 +0900 Subject: MySQL: Fix schema dumping `enum` and `set` columns correctly `enum` and `set` are typed cast as `:string`, but currently the `:string` type is incorrectly reused for schema dumping. A cast type on columns is not always the same with `sql_type`, this fixes schema dumping `enum` and `set` columns to use `sql_type` instead of `type` correctly. --- .../test/cases/adapters/mysql2/enum_test.rb | 14 ++++++++++ .../test/cases/adapters/mysql2/set_test.rb | 32 ++++++++++++++++++++++ .../connection_adapters/mysql_type_lookup_test.rb | 2 +- activerecord/test/schema/mysql2_specific_schema.rb | 4 --- 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 activerecord/test/cases/adapters/mysql2/set_test.rb (limited to 'activerecord/test') diff --git a/activerecord/test/cases/adapters/mysql2/enum_test.rb b/activerecord/test/cases/adapters/mysql2/enum_test.rb index 832f5d61d1..1168b3677e 100644 --- a/activerecord/test/cases/adapters/mysql2/enum_test.rb +++ b/activerecord/test/cases/adapters/mysql2/enum_test.rb @@ -1,11 +1,20 @@ # frozen_string_literal: true require "cases/helper" +require "support/schema_dumping_helper" class Mysql2EnumTest < ActiveRecord::Mysql2TestCase + include SchemaDumpingHelper + class EnumTest < ActiveRecord::Base end + def setup + EnumTest.connection.create_table :enum_tests, id: false, force: true do |t| + t.column :enum_column, "enum('text','blob','tiny','medium','long','unsigned','bigint')" + end + end + def test_enum_limit column = EnumTest.columns_hash["enum_column"] assert_equal 8, column.limit @@ -20,4 +29,9 @@ class Mysql2EnumTest < ActiveRecord::Mysql2TestCase column = EnumTest.columns_hash["enum_column"] assert_not_predicate column, :bigint? end + + def test_schema_dumping + schema = dump_table_schema "enum_tests" + assert_match %r{t\.column "enum_column", "enum\('text','blob','tiny','medium','long','unsigned','bigint'\)"$}, schema + end end diff --git a/activerecord/test/cases/adapters/mysql2/set_test.rb b/activerecord/test/cases/adapters/mysql2/set_test.rb new file mode 100644 index 0000000000..89107e142f --- /dev/null +++ b/activerecord/test/cases/adapters/mysql2/set_test.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "cases/helper" +require "support/schema_dumping_helper" + +class Mysql2SetTest < ActiveRecord::Mysql2TestCase + include SchemaDumpingHelper + + class SetTest < ActiveRecord::Base + end + + def setup + SetTest.connection.create_table :set_tests, id: false, force: true do |t| + t.column :set_column, "set('text','blob','tiny','medium','long','unsigned','bigint')" + end + end + + def test_should_not_be_unsigned + column = SetTest.columns_hash["set_column"] + assert_not_predicate column, :unsigned? + end + + def test_should_not_be_bigint + column = SetTest.columns_hash["set_column"] + assert_not_predicate column, :bigint? + end + + def test_schema_dumping + schema = dump_table_schema "set_tests" + assert_match %r{t\.column "set_column", "set\('text','blob','tiny','medium','long','unsigned','bigint'\)"$}, schema + end +end diff --git a/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb b/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb index bc823fd072..774380d7e0 100644 --- a/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb +++ b/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb @@ -40,7 +40,7 @@ if current_adapter?(:Mysql2Adapter) end def test_enum_type_with_value_matching_other_type - assert_lookup_type :string, "ENUM('unicode', '8bit', 'none')" + assert_lookup_type :string, "ENUM('unicode', '8bit', 'none', 'time')" end def test_binary_types diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index b143035213..911ac808c6 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -62,10 +62,6 @@ ActiveRecord::Schema.define do t.binary :binary_column, limit: 1 end - create_table :enum_tests, id: false, force: true do |t| - t.column :enum_column, "ENUM('text','blob','tiny','medium','long','unsigned','bigint')" - end - execute "DROP PROCEDURE IF EXISTS ten" execute <<~SQL -- cgit v1.2.3 From dddb331bd24ab163ac61b4af7abbdb920264bf9b Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 8 Jul 2019 08:46:07 +0900 Subject: Do not use aliases in GROUP BY clause It appears that Oracle does not allow using aliases in GROUP BY clause unlike ORDER BY clause. Fixes #36613. --- activerecord/test/cases/calculations_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 525085bb28..dbd1d03c4c 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -139,6 +139,13 @@ class CalculationsTest < ActiveRecord::TestCase end end + def test_should_not_use_alias_for_grouped_field + assert_sql(/GROUP BY #{Regexp.escape(Account.connection.quote_table_name("accounts.firm_id"))}/i) do + c = Account.group(:firm_id).order("accounts_firm_id").sum(:credit_limit) + assert_equal [1, 2, 6, 9], c.keys.compact + end + end + def test_should_order_by_grouped_field c = Account.group(:firm_id).order("firm_id").sum(:credit_limit) assert_equal [1, 2, 6, 9], c.keys.compact -- cgit v1.2.3 From 7d699dad334996838dd529e4b75e1648692d56f8 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 8 Jul 2019 09:26:03 +0900 Subject: Should `Regexp.escape` quoted table name in regex It is for agnostic test case, since quoted table name may include `.` for all adapters, and `[` / `]` for sqlserver adapter. --- .../test/cases/adapters/mysql2/annotate_test.rb | 37 ----------------- .../cases/adapters/postgresql/annotate_test.rb | 37 ----------------- .../test/cases/adapters/sqlite3/annotate_test.rb | 37 ----------------- activerecord/test/cases/annotate_test.rb | 46 ++++++++++++++++++++++ activerecord/test/cases/batches_test.rb | 2 +- activerecord/test/cases/finder_test.rb | 3 +- activerecord/test/cases/inheritance_test.rb | 4 +- 7 files changed, 51 insertions(+), 115 deletions(-) delete mode 100644 activerecord/test/cases/adapters/mysql2/annotate_test.rb delete mode 100644 activerecord/test/cases/adapters/postgresql/annotate_test.rb delete mode 100644 activerecord/test/cases/adapters/sqlite3/annotate_test.rb create mode 100644 activerecord/test/cases/annotate_test.rb (limited to 'activerecord/test') diff --git a/activerecord/test/cases/adapters/mysql2/annotate_test.rb b/activerecord/test/cases/adapters/mysql2/annotate_test.rb deleted file mode 100644 index b512540073..0000000000 --- a/activerecord/test/cases/adapters/mysql2/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class Mysql2AnnotateTest < ActiveRecord::Mysql2TestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/adapters/postgresql/annotate_test.rb b/activerecord/test/cases/adapters/postgresql/annotate_test.rb deleted file mode 100644 index 42a2861511..0000000000 --- a/activerecord/test/cases/adapters/postgresql/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class PostgresqlAnnotateTest < ActiveRecord::PostgreSQLTestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/adapters/sqlite3/annotate_test.rb b/activerecord/test/cases/adapters/sqlite3/annotate_test.rb deleted file mode 100644 index 6567a5eca3..0000000000 --- a/activerecord/test/cases/adapters/sqlite3/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class SQLite3AnnotateTest < ActiveRecord::SQLite3TestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/annotate_test.rb b/activerecord/test/cases/annotate_test.rb new file mode 100644 index 0000000000..4d71d28f83 --- /dev/null +++ b/activerecord/test/cases/annotate_test.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" + +class AnnotateTest < ActiveRecord::TestCase + fixtures :posts + + def test_annotate_wraps_content_in_an_inline_comment + quoted_posts_id, quoted_posts = regexp_escape_table_name("posts.id"), regexp_escape_table_name("posts") + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("foo") + assert posts.first + end + end + + def test_annotate_is_sanitized + quoted_posts_id, quoted_posts = regexp_escape_table_name("posts.id"), regexp_escape_table_name("posts") + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("*/foo/*") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("**//foo//**") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/ /\* bar \*/}i) do + posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* \+ MAX_EXECUTION_TIME\(1\) \*/}i) do + posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") + assert posts.first + end + end + + private + def regexp_escape_table_name(name) + Regexp.escape(Post.connection.quote_table_name(name)) + end +end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index cf6e280898..0d0bf39f79 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -146,7 +146,7 @@ class EachTest < ActiveRecord::TestCase def test_find_in_batches_should_quote_batch_order c = Post.connection - assert_sql(/ORDER BY #{c.quote_table_name('posts')}\.#{c.quote_column_name('id')}/) do + assert_sql(/ORDER BY #{Regexp.escape(c.quote_table_name("posts.id"))}/i) do Post.find_in_batches(batch_size: 1) do |batch| assert_kind_of Array, batch assert_kind_of Post, batch.first diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 3aa610f86b..3752fd42e3 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -245,7 +245,8 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_does_not_select_columns_without_alias - assert_sql(/SELECT\W+1 AS one FROM ["`]topics["`]/i) do + c = Topic.connection + assert_sql(/SELECT 1 AS one FROM #{Regexp.escape(c.quote_table_name("topics"))}/i) do Topic.exists? end end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 629167e9ed..01e4878c3f 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -471,9 +471,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_eager_load_belongs_to_primary_key_quoting - con = Account.connection + c = Account.connection bind_param = Arel::Nodes::BindParam.new(nil) - assert_sql(/#{con.quote_table_name('companies')}\.#{con.quote_column_name('id')} = (?:#{Regexp.quote(bind_param.to_sql)}|1)/) do + assert_sql(/#{Regexp.escape(c.quote_table_name("companies.id"))} = (?:#{Regexp.escape(bind_param.to_sql)}|1)/i) do Account.all.merge!(includes: :firm).find(1) end end -- cgit v1.2.3 From ff42b21915c08cf44f2c263dfe13c249dd780528 Mon Sep 17 00:00:00 2001 From: Will Jessop Date: Mon, 8 Jul 2019 05:49:58 +0100 Subject: When DATABASE_URL is specified don't trample envs that use a url: key fixes #36610 --- .../merge_and_resolve_default_url_config_test.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index c0a9f8f9ca..6372abbf3f 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -244,6 +244,25 @@ module ActiveRecord assert_equal expected, actual end + def test_no_url_sub_key_with_database_url_doesnt_trample_other_envs + ENV["DATABASE_URL"] = "postgres://localhost/baz" + + config = { "default_env" => { "database" => "foo" }, "other_env" => { "url" => "postgres://foohost/bardb" } } + actual = resolve_config(config) + expected = { "default_env" => + { "database" => "baz", + "adapter" => "postgresql", + "host" => "localhost" + }, + "other_env" => + { "adapter" => "postgresql", + "database" => "bardb", + "host" => "foohost" + } + } + assert_equal expected, actual + end + def test_merge_no_conflicts_with_database_url ENV["DATABASE_URL"] = "postgres://localhost/foo" -- cgit v1.2.3