From 9694f86de65e9162a6802a43cdbce9ccc85285b5 Mon Sep 17 00:00:00 2001 From: Prathamesh Sonpatki Date: Mon, 29 Jul 2013 09:22:14 +0530 Subject: [Active Record] Renamed private methods create_record and update_record This is to ensure that they are not accidentally called by the app code. They are renamed to _create_record and _update_record respectively. Closes #11645 --- .../cases/associations/belongs_to_associations_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 9340bc0a83..d172ee2e7a 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -863,4 +863,22 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end end end + + test 'belongs_to works with model name Record' do + Record = Class.new(ActiveRecord::Base) do + connection.create_table :records + end + + Foo = Class.new(ActiveRecord::Base) do + connection.create_table :foos do |t| + t.belongs_to :record + end + + belongs_to :record + end + + record = Record.create! + Foo.create! record: record + assert_equal 1, Foo.count + end end -- cgit v1.2.3 From 1b187caaa1e1aa1bd0f440052b4df09a5ddaa4bf Mon Sep 17 00:00:00 2001 From: Jefferson Lai Date: Tue, 1 Apr 2014 20:18:16 -0700 Subject: CollectionProxy uses the arel of its association's scope. CollectionProxy should be able to reuse the behavior (methods) of its parent class, but with its own state. This change allows CollectionProxy to use the arel object corresponding to its association's scope. --- activerecord/test/cases/relations_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index fddb7c204a..049c5a0606 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -573,6 +573,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, actual end + def test_to_sql_on_scoped_proxy + auth = Author.first + Post.where("1=1").written_by(auth) + assert_not auth.posts.to_sql.include?("1=1") + end + def test_loading_with_one_association_with_non_preload posts = Post.eager_load(:last_comment).order('comments.id DESC') post = posts.find { |p| p.id == 1 } -- cgit v1.2.3 From f4226c3ab6651f6871e02f3c6754c29ab155b938 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Thu, 3 Apr 2014 14:59:53 +0200 Subject: PostgreSQL and SQLite, remove varchar limit. [Vladimir Sazhin & Toms Mikoss & Yves Senn] There is no reason for the PG adapter to have a default limit of 255 on :string columns. See this snippet from the PG docs: Tip: There is no performance difference among these three types, apart from increased storage space when using the blank-padded type, and a few extra CPU cycles to check the length when storing into a length-constrained column. While character(n) has performance advantages in some other database systems, there is no such advantage in PostgreSQL; in fact character(n) is usually the slowest of the three because of its additional storage costs. In most situations text or character varying should be used instead. --- activerecord/test/cases/adapters/postgresql/array_test.rb | 2 +- activerecord/test/cases/reflection_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 36ded66998..18dd4a6de8 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -25,7 +25,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_column assert_equal :string, @column.type - assert_equal "character varying(255)", @column.sql_type + assert_equal "character varying", @column.sql_type assert @column.array assert_not @column.text? assert_not @column.number? diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index d7ad5ed29f..ad77472333 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -63,7 +63,7 @@ class ReflectionTest < ActiveRecord::TestCase def test_column_string_type_and_limit assert_equal :string, @first.column_for_attribute("title").type - assert_equal 255, @first.column_for_attribute("title").limit + assert_equal 250, @first.column_for_attribute("title").limit end def test_column_null_not_null -- cgit v1.2.3 From 80f4a65bbd2372c01cfdf174e6446cd232d81e44 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 4 Apr 2014 09:16:02 +0200 Subject: test, show current adapter behavior for `add_column limit: nil`. This is an illustration of https://github.com/rails/rails/pull/13435#issuecomment-33789752 Removing the limit from the PG and SQLite adapter solves the issue. MySQL is still affected by the issue. --- activerecord/test/cases/migration/column_attributes_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb index ccf19fb4d0..6a02873cba 100644 --- a/activerecord/test/cases/migration/column_attributes_test.rb +++ b/activerecord/test/cases/migration/column_attributes_test.rb @@ -35,6 +35,14 @@ module ActiveRecord assert_no_column TestModel, :last_name end + def test_add_column_without_limit + # TODO: limit: nil should work with all adapters. + skip "MySQL wrongly enforces a limit of 255" if current_adapter?(:MysqlAdapter, :Mysql2Adapter) + add_column :test_models, :description, :string, limit: nil + TestModel.reset_column_information + assert_nil TestModel.columns_hash["description"].limit + end + if current_adapter?(:MysqlAdapter, :Mysql2Adapter) def test_unabstracted_database_dependent_types add_column :test_models, :intelligence_quotient, :tinyint -- cgit v1.2.3 From 57c7d5cb80086e66eee3303b11a5e558aa9e5bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 4 Apr 2014 19:44:17 -0300 Subject: Fix the test defining the models in the right place --- .../associations/belongs_to_associations_test.rb | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 091c94676e..27f6fa575d 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -16,6 +16,8 @@ require 'models/essay' require 'models/toy' require 'models/invoice' require 'models/line_item' +require 'models/column' +require 'models/record' class BelongsToAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :topics, @@ -886,21 +888,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end end - test 'belongs_to works with model name Record' do - Record = Class.new(ActiveRecord::Base) do - connection.create_table :records - end - - Foo = Class.new(ActiveRecord::Base) do - connection.create_table :foos do |t| - t.belongs_to :record - end - - belongs_to :record - end - + test 'belongs_to works with model called Record' do record = Record.create! - Foo.create! record: record - assert_equal 1, Foo.count + Column.create! record: record + assert_equal 1, Column.count end end -- cgit v1.2.3 From bbe7fe41692e5a2c869f812616d0c99a2bfcb39c Mon Sep 17 00:00:00 2001 From: Evan Whalen Date: Mon, 7 Apr 2014 10:01:03 -0400 Subject: make enums distinct per class --- activerecord/test/cases/enum_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 47de3dec98..5157c272ca 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -250,4 +250,19 @@ class EnumTest < ActiveRecord::TestCase valid_book = klass.new(status: "written") assert valid_book.valid? end + + test "enums are distinct per class" do + Plane = Class.new(ActiveRecord::Base) do + enum status: [:grounded, :operational] + end + assert_equal({ "proposed" => 0, "written" => 1, "published" => 2 }, Book.statuses) + assert_equal({ "grounded" => 0, "operational" => 1 }, Plane.statuses) + end + + test "enums are inheritable" do + Encyclopedia = Class.new(Book) do + enum status: [:published, :reprinted] + end + assert_equal({ "published" => 0, "reprinted" => 1 }, Encyclopedia.statuses) + end end -- cgit v1.2.3 From bbad7523f08936f38938c7d4ff18f4084f008244 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Mon, 7 Apr 2014 11:07:46 -0300 Subject: Ignore order when doing count. This is necessary because Postgresql doesn't play nice with ORDER BY and no GROUP BY. Fixes #14621. --- activerecord/test/cases/calculations_test.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index db999f90ab..b8de78934e 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -387,6 +387,20 @@ class CalculationsTest < ActiveRecord::TestCase assert_raise(ArgumentError) { Account.count(1, 2, 3) } end + def test_count_with_order + assert_equal 6, Account.order(:credit_limit).count + end + + def test_count_with_reverse_order + assert_equal 6, Account.order(:credit_limit).reverse_order.count + end + + def test_count_with_where_and_order + assert_equal 1, Account.where(firm_name: '37signals').count + assert_equal 1, Account.where(firm_name: '37signals').order(:firm_name).count + assert_equal 1, Account.where(firm_name: '37signals').order(:firm_name).reverse_order.count + end + def test_should_sum_expression # Oracle adapter returns floating point value 636.0 after SUM if current_adapter?(:OracleAdapter) -- cgit v1.2.3 From c4bdca19a7a61eb12c75c4a3225e54b69b486a15 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Mon, 7 Apr 2014 22:54:58 +0930 Subject: Use connection-specific bytea escaping In our normal usage, it's rare for this to make a difference... but is more technically correct. As well as a spec that proves this is a good idea, let's also add a more sane-looking one that just covers basic to_sql functionality. There aren't many places where we actually use escape_bytea, but that's one that won't be going away. --- .../test/cases/adapters/postgresql/bytea_test.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb index c3394d7712..84fa199f17 100644 --- a/activerecord/test/cases/adapters/postgresql/bytea_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb @@ -70,6 +70,23 @@ class PostgresqlByteaTest < ActiveRecord::TestCase assert_equal(data, record.payload) end + def test_via_to_sql + data = "'\u001F\\" + record = ByteaDataType.create(payload: data) + sql = ByteaDataType.where(payload: data).select(:payload).to_sql + result = @connection.query(sql) + assert_equal([[data]], result) + end + + def test_via_to_sql_with_complicating_connection + Thread.new do + other_conn = ActiveRecord::Base.connection + other_conn.execute('SET standard_conforming_strings = off') + end.join + + test_via_to_sql + end + def test_write_binary data = File.read(File.join(File.dirname(__FILE__), '..', '..', '..', 'assets', 'example.log')) assert(data.size > 1) -- cgit v1.2.3 From 6c311e0b7538e8c55797aa889fdf66780ab283a4 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Mon, 7 Apr 2014 16:01:17 -0300 Subject: Build the reverse_order on its proper method. The reverse_order method was using a flag to control if the order should be reversed or not. Instead of using this variable just build the reverse order inside its proper method. This implementation was leading to an unexpected behavior when using reverse_order and then applying reorder(nil). Example: Before Post.order(:name).reverse_order.reorder(nil) # => SELECT "posts".* FROM "posts" ORDER BY "posts"."id" DESC After Post.order(:name).reverse_order.reorder(nil) # => SELECT "posts".* FROM "posts" --- activerecord/test/cases/relation/mutation_test.rb | 14 +++++++++++--- activerecord/test/cases/relations_test.rb | 12 ++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 4fafa668fb..c81a3002d6 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -107,10 +107,18 @@ module ActiveRecord end test 'reverse_order!' do - assert relation.reverse_order!.equal?(relation) - assert relation.reverse_order_value + relation = Post.order('title ASC, comments_count DESC') + + relation.reverse_order! + + assert_equal 'title DESC', relation.order_values.first + assert_equal 'comments_count ASC', relation.order_values.last + + relation.reverse_order! - assert !relation.reverse_order_value + + assert_equal 'title ASC', relation.order_values.first + assert_equal 'comments_count DESC', relation.order_values.last end test 'create_with!' do diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 049c5a0606..2aa6d643a5 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1424,6 +1424,18 @@ class RelationTest < ActiveRecord::TestCase assert_equal [], scope.references_values end + def test_order_with_reorder_nil_removes_the_order + relation = Post.order(:title).reorder(nil) + + assert_nil relation.order_values.first + end + + def test_reverse_order_with_reorder_nil_removes_the_order + relation = Post.order(:title).reverse_order.reorder(nil) + + assert_nil relation.order_values.first + end + def test_presence topics = Topic.all -- cgit v1.2.3 From a5664fe27b1797983537c0764003e618bd3d2801 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 7 Apr 2014 18:52:21 -0700 Subject: Follow up to bbe7fe41 to fix enum leakage across classes. The original attempt didn't really fix the problem and wasn't testing the problematic area. This commit corrected those issues in the original commit. Also removed the private `enum_mapping_for` method. As `defined_enums` is now a method, this method doesn't provide much value anymore. --- activerecord/test/cases/enum_test.rb | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 5157c272ca..3b2f0dfe07 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -252,17 +252,38 @@ class EnumTest < ActiveRecord::TestCase end test "enums are distinct per class" do - Plane = Class.new(ActiveRecord::Base) do - enum status: [:grounded, :operational] + klass1 = Class.new(ActiveRecord::Base) do + self.table_name = "books" + enum status: [:proposed, :written] + end + + klass2 = Class.new(ActiveRecord::Base) do + self.table_name = "books" + enum status: [:drafted, :uploaded] end - assert_equal({ "proposed" => 0, "written" => 1, "published" => 2 }, Book.statuses) - assert_equal({ "grounded" => 0, "operational" => 1 }, Plane.statuses) + + book1 = klass1.proposed.create! + book1.status = :written + assert_equal ['proposed', 'written'], book1.status_change + + book2 = klass2.drafted.create! + book2.status = :uploaded + assert_equal ['drafted', 'uploaded'], book2.status_change end test "enums are inheritable" do - Encyclopedia = Class.new(Book) do - enum status: [:published, :reprinted] + subklass1 = Class.new(Book) + + subklass2 = Class.new(Book) do + enum status: [:drafted, :uploaded] end - assert_equal({ "published" => 0, "reprinted" => 1 }, Encyclopedia.statuses) + + book1 = subklass1.proposed.create! + book1.status = :written + assert_equal ['proposed', 'written'], book1.status_change + + book2 = subklass2.drafted.create! + book2.status = :uploaded + assert_equal ['drafted', 'uploaded'], book2.status_change end end -- cgit v1.2.3 From 2b817cde622e625f08a638d909f3e9b12f0b3066 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Tue, 8 Apr 2014 03:54:23 +0930 Subject: Only apply DATABASE_URL for Rails.env As we like ENV vars, also support DATABASE_URL_#{env}, for more obscure use cases. --- .../connection_adapters/connection_handler_test.rb | 43 +++++++++++++--------- 1 file changed, 26 insertions(+), 17 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index e097449029..ed7188e2e0 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -27,28 +27,36 @@ module ActiveRecord def test_resolver_with_database_uri_and_known_key ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:production, config) + config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } + actual = resolve(:default_env, config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end def test_resolver_with_database_uri_and_known_string_key ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = assert_deprecated { resolve("production", config) } + config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } + actual = assert_deprecated { resolve("default_env", config) } expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end - def test_resolver_with_database_uri_and_unknown_symbol_key + def test_resolver_with_database_uri_and_current_env_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:production, config) + actual = resolve(:default_env, config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end + def test_resolver_with_database_uri_and_unknown_symbol_key + ENV['DATABASE_URL'] = "postgres://localhost/foo" + config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } + assert_raises AdapterNotSpecified do + resolve(:production, config) + end + end + def test_resolver_with_database_uri_and_unknown_string_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } @@ -73,10 +81,10 @@ module ActiveRecord def test_environment_does_not_exist_in_config_url_does_exist ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } + config = { "not_default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = klass.new(config).resolve expect_prod = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } - assert_equal expect_prod, actual["production"] + assert_equal expect_prod, actual["default_env"] end def test_string_connection @@ -123,9 +131,10 @@ module ActiveRecord expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } - assert_equal expected, actual["production"] - assert_equal expected, actual["development"] - assert_equal expected, actual["test"] + assert_equal expected, actual["default_env"] + assert_equal nil, actual["production"] + assert_equal nil, actual["development"] + assert_equal nil, actual["test"] assert_equal nil, actual[:production] assert_equal nil, actual[:development] assert_equal nil, actual[:test] @@ -134,9 +143,9 @@ module ActiveRecord def test_url_sub_key_with_database_url ENV['DATABASE_URL'] = "NOT-POSTGRES://localhost/NOT_FOO" - config = { "production" => { "url" => "postgres://localhost/foo" } } + config = { "default_env" => { "url" => "postgres://localhost/foo" } } actual = klass.new(config).resolve - expected = { "production" => + expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" @@ -148,9 +157,9 @@ module ActiveRecord def test_merge_no_conflicts_with_database_url ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = {"production" => { "pool" => "5" } } + config = {"default_env" => { "pool" => "5" } } actual = klass.new(config).resolve - expected = { "production" => + expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", "host" => "localhost", @@ -163,9 +172,9 @@ module ActiveRecord def test_merge_conflicts_with_database_url ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = {"production" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } } + config = {"default_env" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } } actual = klass.new(config).resolve - expected = { "production" => + expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", "host" => "localhost", -- cgit v1.2.3 From e533826f483d6a1f45d044a2017ff4e9d19a154e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 7 Apr 2014 19:05:50 -0300 Subject: Make sure DEFAULT_URL only override current environment database configuration --- .../connection_adapters/connection_handler_test.rb | 38 +++++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index ed7188e2e0..aec70fac79 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -11,10 +11,14 @@ module ActiveRecord def setup @previous_database_url = ENV.delete("DATABASE_URL") + @previous_database_url_default_env = ENV.delete("DATABASE_URL_DEFAULT_ENV") + @previous_database_url_production = ENV.delete("DATABASE_URL_PRODUCTION") end teardown do ENV["DATABASE_URL"] = @previous_database_url + ENV["DATABASE_URL_DEFAULT_ENV"] = @previous_database_url_default_env + ENV["DATABASE_URL_PRODUCTION"] = @previous_database_url_production end def resolve(spec, config) @@ -25,15 +29,23 @@ module ActiveRecord ConnectionSpecification::Resolver.new(klass.new(config).resolve).spec(spec) end - def test_resolver_with_database_uri_and_known_key + def test_resolver_with_database_uri_and_current_env_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" + config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } + actual = resolve(:default_env, config) + expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } + assert_equal expected, actual + end + + def test_resolver_with_environment_database_uri_and_current_env_symbol_key + ENV['DATABASE_URL_DEFAULT_ENV'] = "postgres://localhost/foo" config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = resolve(:default_env, config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end - def test_resolver_with_database_uri_and_known_string_key + def test_resolver_with_database_uri_and_and_current_env_string_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = assert_deprecated { resolve("default_env", config) } @@ -41,10 +53,18 @@ module ActiveRecord assert_equal expected, actual end - def test_resolver_with_database_uri_and_current_env_symbol_key + def test_resolver_with_database_uri_and_known_key ENV['DATABASE_URL'] = "postgres://localhost/foo" - config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:default_env, config) + config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } + actual = resolve(:production, config) + expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost" } + assert_equal expected, actual + end + + def test_resolver_with_custom_database_uri_and_custom_key + ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" + config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } + actual = resolve(:production, config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end @@ -88,9 +108,9 @@ module ActiveRecord end def test_string_connection - config = { "production" => "postgres://localhost/foo" } + config = { "default_env" => "postgres://localhost/foo" } actual = klass.new(config).resolve - expected = { "production" => + expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" @@ -100,9 +120,9 @@ module ActiveRecord end def test_url_sub_key - config = { "production" => { "url" => "postgres://localhost/foo" } } + config = { "default_env" => { "url" => "postgres://localhost/foo" } } actual = klass.new(config).resolve - expected = { "production" => + expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" -- cgit v1.2.3 From d459f751c934529e6a3cff36554a02d6ce7666f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 7 Apr 2014 19:16:20 -0300 Subject: Test DATABASE_URL has precendance over DATABASE_URL_#{environment} --- .../test/cases/connection_adapters/connection_handler_test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index aec70fac79..ae4172f15c 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -45,6 +45,15 @@ module ActiveRecord assert_equal expected, actual end + def test_resolver_with_environment_database_uri_and_global_database_uri_and_current_env_symbol_key + ENV['DATABASE_URL'] = "postgres://localhost/foo" + ENV['DATABASE_URL_DEFAULT_ENV'] = "mysql://host/foo_bar" + config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } + actual = resolve(:default_env, config) + expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } + assert_equal expected, actual + end + def test_resolver_with_database_uri_and_and_current_env_string_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } -- cgit v1.2.3 From 23692184bbcc2e411ed6bc6d1eaea09aa0f0474f Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Tue, 8 Apr 2014 15:00:38 +0930 Subject: Give a deprecation message even when the lookup fails If the supplied string doesn't contain a colon, it clearly cannot be a database URL. They must have intended to do a key lookup, so even though it failed, give the explanatory deprecation warning, and raise the exception that lists the known configs. Conveniently, this also simplifies our logical behaviour: if the string matches a known configuration, or doesn't contain a colon (and is therefore clearly not a URL), then we output a deprecation warning, and behave exactly as we would if it were a symbol. --- .../test/cases/connection_adapters/connection_handler_test.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index ae4172f15c..dd03f57f19 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -89,8 +89,10 @@ module ActiveRecord def test_resolver_with_database_uri_and_unknown_string_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - assert_raises AdapterNotSpecified do - spec("production", config) + assert_deprecated do + assert_raises AdapterNotSpecified do + spec("production", config) + end end end -- cgit v1.2.3 From 8f23c220081d0197107490df9aa6e262fac5099e Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Tue, 8 Apr 2014 15:10:15 +0930 Subject: Ensure we correctly and immediately load all ENV entries .. even when the supplied config made no hint that name was relevant. --- .../connection_adapters/connection_handler_test.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index dd03f57f19..d6abc13c0d 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -70,6 +70,14 @@ module ActiveRecord assert_equal expected, actual end + def test_resolver_with_custom_database_uri_and_no_matching_config + ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" + config = {} + actual = resolve(:production, config) + expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } + assert_equal expected, actual + end + def test_resolver_with_custom_database_uri_and_custom_key ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } @@ -118,6 +126,19 @@ module ActiveRecord assert_equal expect_prod, actual["default_env"] end + def test_environment_key_available_immediately + ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" + config = {} + actual = klass.new(config).resolve + expected = { "production" => + { "adapter" => "postgresql", + "database" => "foo", + "host" => "localhost" + } + } + assert_equal expected, actual + end + def test_string_connection config = { "default_env" => "postgres://localhost/foo" } actual = klass.new(config).resolve -- cgit v1.2.3 From 615e0dcdf1391a8b71ad6556c2e7b9cedf6ffa12 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Wed, 9 Apr 2014 00:17:13 +0930 Subject: Less ambition, more deprecation The "DATABASE_URL_*" idea was moving in the wrong direction. Instead, let's deprecate the situation where we end up using ENV['DATABASE_URL'] at all: the Right Way is to explicitly include it in database.yml with ERB. --- .../connection_adapters/connection_handler_test.rb | 70 ++++------------------ 1 file changed, 11 insertions(+), 59 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index d6abc13c0d..a7002bd13d 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -11,14 +11,10 @@ module ActiveRecord def setup @previous_database_url = ENV.delete("DATABASE_URL") - @previous_database_url_default_env = ENV.delete("DATABASE_URL_DEFAULT_ENV") - @previous_database_url_production = ENV.delete("DATABASE_URL_PRODUCTION") end teardown do ENV["DATABASE_URL"] = @previous_database_url - ENV["DATABASE_URL_DEFAULT_ENV"] = @previous_database_url_default_env - ENV["DATABASE_URL_PRODUCTION"] = @previous_database_url_production end def resolve(spec, config) @@ -32,24 +28,7 @@ module ActiveRecord def test_resolver_with_database_uri_and_current_env_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:default_env, config) - expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } - assert_equal expected, actual - end - - def test_resolver_with_environment_database_uri_and_current_env_symbol_key - ENV['DATABASE_URL_DEFAULT_ENV'] = "postgres://localhost/foo" - config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:default_env, config) - expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } - assert_equal expected, actual - end - - def test_resolver_with_environment_database_uri_and_global_database_uri_and_current_env_symbol_key - ENV['DATABASE_URL'] = "postgres://localhost/foo" - ENV['DATABASE_URL_DEFAULT_ENV'] = "mysql://host/foo_bar" - config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve(:default_env, config) + actual = assert_deprecated { resolve(:default_env, config) } expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end @@ -65,32 +44,18 @@ module ActiveRecord def test_resolver_with_database_uri_and_known_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } - actual = resolve(:production, config) + actual = assert_deprecated { resolve(:production, config) } expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost" } assert_equal expected, actual end - def test_resolver_with_custom_database_uri_and_no_matching_config - ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" - config = {} - actual = resolve(:production, config) - expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } - assert_equal expected, actual - end - - def test_resolver_with_custom_database_uri_and_custom_key - ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" - config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } - actual = resolve(:production, config) - expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } - assert_equal expected, actual - end - def test_resolver_with_database_uri_and_unknown_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - assert_raises AdapterNotSpecified do - resolve(:production, config) + assert_deprecated do + assert_raises AdapterNotSpecified do + resolve(:production, config) + end end end @@ -107,7 +72,7 @@ module ActiveRecord def test_resolver_with_database_uri_and_supplied_url ENV['DATABASE_URL'] = "not-postgres://not-localhost/not_foo" config = { "production" => { "adapter" => "also_not_postgres", "database" => "also_not_foo" } } - actual = resolve("postgres://localhost/foo", config) + actual = assert_deprecated { resolve("postgres://localhost/foo", config) } expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end @@ -121,24 +86,11 @@ module ActiveRecord def test_environment_does_not_exist_in_config_url_does_exist ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = klass.new(config).resolve + actual = assert_deprecated { klass.new(config).resolve } expect_prod = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expect_prod, actual["default_env"] end - def test_environment_key_available_immediately - ENV['DATABASE_URL_PRODUCTION'] = "postgres://localhost/foo" - config = {} - actual = klass.new(config).resolve - expected = { "production" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" - } - } - assert_equal expected, actual - end - def test_string_connection config = { "default_env" => "postgres://localhost/foo" } actual = klass.new(config).resolve @@ -179,7 +131,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {} - actual = klass.new(config).resolve + actual = assert_deprecated { klass.new(config).resolve } expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } @@ -210,7 +162,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {"default_env" => { "pool" => "5" } } - actual = klass.new(config).resolve + actual = assert_deprecated { klass.new(config).resolve } expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", @@ -225,7 +177,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {"default_env" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } } - actual = klass.new(config).resolve + actual = assert_deprecated { klass.new(config).resolve } expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", -- cgit v1.2.3 From 88cd65b174d9a75d9cb0e5e06a57615ccc80fff1 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Wed, 9 Apr 2014 00:55:49 +0930 Subject: Don't deprecate after all --- .../connection_adapters/connection_handler_test.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index a7002bd13d..256ddd7d23 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -28,7 +28,7 @@ module ActiveRecord def test_resolver_with_database_uri_and_current_env_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = assert_deprecated { resolve(:default_env, config) } + actual = resolve(:default_env, config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end @@ -44,7 +44,7 @@ module ActiveRecord def test_resolver_with_database_uri_and_known_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } - actual = assert_deprecated { resolve(:production, config) } + actual = resolve(:production, config) expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost" } assert_equal expected, actual end @@ -52,10 +52,8 @@ module ActiveRecord def test_resolver_with_database_uri_and_unknown_symbol_key ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } - assert_deprecated do - assert_raises AdapterNotSpecified do - resolve(:production, config) - end + assert_raises AdapterNotSpecified do + resolve(:production, config) end end @@ -72,7 +70,7 @@ module ActiveRecord def test_resolver_with_database_uri_and_supplied_url ENV['DATABASE_URL'] = "not-postgres://not-localhost/not_foo" config = { "production" => { "adapter" => "also_not_postgres", "database" => "also_not_foo" } } - actual = assert_deprecated { resolve("postgres://localhost/foo", config) } + actual = resolve("postgres://localhost/foo", config) expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expected, actual end @@ -86,7 +84,7 @@ module ActiveRecord def test_environment_does_not_exist_in_config_url_does_exist ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = assert_deprecated { klass.new(config).resolve } + actual = klass.new(config).resolve expect_prod = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } assert_equal expect_prod, actual["default_env"] end @@ -131,7 +129,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {} - actual = assert_deprecated { klass.new(config).resolve } + actual = klass.new(config).resolve expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } @@ -162,7 +160,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {"default_env" => { "pool" => "5" } } - actual = assert_deprecated { klass.new(config).resolve } + actual = klass.new(config).resolve expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", @@ -177,7 +175,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = {"default_env" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } } - actual = assert_deprecated { klass.new(config).resolve } + actual = klass.new(config).resolve expected = { "default_env" => { "adapter" => "postgresql", "database" => "foo", -- cgit v1.2.3 From d72a0cbc807a14d3eec02a53317d11b9d9fa5815 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Wed, 9 Apr 2014 01:41:11 +0930 Subject: Drop in @jeremy's new database.yml template text In passing, allow multi-word adapters to be referenced in a URL: underscored_name must become hyphened-name. --- .../test/cases/connection_adapters/connection_handler_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 256ddd7d23..f2d18e812d 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -89,6 +89,14 @@ module ActiveRecord assert_equal expect_prod, actual["default_env"] end + def test_url_with_hyphenated_scheme + ENV['DATABASE_URL'] = "ibm-db://localhost/foo" + config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } + actual = resolve(:default_env, config) + expected = { "adapter"=>"ibm_db", "database"=>"foo", "host"=>"localhost" } + assert_equal expected, actual + end + def test_string_connection config = { "default_env" => "postgres://localhost/foo" } actual = klass.new(config).resolve -- cgit v1.2.3 From 213ef567ae2ab92f1e1145cd385da0c5b9534422 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Wed, 9 Apr 2014 21:56:40 -0300 Subject: Make sure the reflection test is passing a String to the reflection cache. --- activerecord/test/cases/reflection_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index ad77472333..fed199f6e9 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -192,7 +192,7 @@ class ReflectionTest < ActiveRecord::TestCase end def test_reflection_should_not_raise_error_when_compared_to_other_object - assert_nothing_raised { Firm.reflections[:clients] == Object.new } + assert_nothing_raised { Firm.reflections['clients'] == Object.new } end def test_has_many_through_reflection -- cgit v1.2.3