From 9694f86de65e9162a6802a43cdbce9ccc85285b5 Mon Sep 17 00:00:00 2001
From: Prathamesh Sonpatki <csonpatki@gmail.com>
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
---
 .../associations/collection_association.rb             |  8 ++++----
 .../active_record/associations/singular_association.rb |  6 +++---
 .../lib/active_record/attribute_methods/dirty.rb       |  4 ++--
 activerecord/lib/active_record/callbacks.rb            |  4 ++--
 activerecord/lib/active_record/locking/optimistic.rb   |  2 +-
 activerecord/lib/active_record/persistence.rb          |  8 ++++----
 activerecord/lib/active_record/relation.rb             |  2 +-
 activerecord/lib/active_record/timestamp.rb            |  4 ++--
 .../cases/associations/belongs_to_associations_test.rb | 18 ++++++++++++++++++
 9 files changed, 37 insertions(+), 19 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 03ca00fa70..b90c90c7c4 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -138,11 +138,11 @@ module ActiveRecord
       end
 
       def create(attributes = {}, &block)
-        create_record(attributes, &block)
+        _create_record(attributes, &block)
       end
 
       def create!(attributes = {}, &block)
-        create_record(attributes, true, &block)
+        _create_record(attributes, true, &block)
       end
 
       # Add +records+ to this association. Returns +self+ so method calls may
@@ -452,13 +452,13 @@ module ActiveRecord
           persisted + memory
         end
 
-        def create_record(attributes, raise = false, &block)
+        def _create_record(attributes, raise = false, &block)
           unless owner.persisted?
             raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
           end
 
           if attributes.is_a?(Array)
-            attributes.collect { |attr| create_record(attr, raise, &block) }
+            attributes.collect { |attr| _create_record(attr, raise, &block) }
           else
             transaction do
               add_to_target(build_record(attributes)) do |record|
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index 399aff378a..747bb5f1d6 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -18,11 +18,11 @@ module ActiveRecord
       end
 
       def create(attributes = {}, &block)
-        create_record(attributes, &block)
+        _create_record(attributes, &block)
       end
 
       def create!(attributes = {}, &block)
-        create_record(attributes, true, &block)
+        _create_record(attributes, true, &block)
       end
 
       def build(attributes = {})
@@ -52,7 +52,7 @@ module ActiveRecord
           replace(record)
         end
 
-        def create_record(attributes, raise_error = false)
+        def _create_record(attributes, raise_error = false)
           record = build_record(attributes)
           yield(record) if block_given?
           saved = record.save
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index 8a1b199997..99070f127b 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -79,11 +79,11 @@ module ActiveRecord
         end
       end
 
-      def update_record(*)
+      def _update_record(*)
         partial_writes? ? super(keys_for_partial_write) : super
       end
 
-      def create_record(*)
+      def _create_record(*)
         partial_writes? ? super(keys_for_partial_write) : super
       end
 
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 35f19f0bc0..5955673b42 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -302,11 +302,11 @@ module ActiveRecord
       run_callbacks(:save) { super }
     end
 
-    def create_record #:nodoc:
+    def _create_record #:nodoc:
       run_callbacks(:create) { super }
     end
 
-    def update_record(*) #:nodoc:
+    def _update_record(*) #:nodoc:
       run_callbacks(:update) { super }
     end
   end
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 6f54729b3c..4d63b04d9f 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -66,7 +66,7 @@ module ActiveRecord
           send(lock_col + '=', previous_lock_value + 1)
         end
 
-        def update_record(attribute_names = @attributes.keys) #:nodoc:
+        def _update_record(attribute_names = @attributes.keys) #:nodoc:
           return super unless locking_enabled?
           return 0 if attribute_names.empty?
 
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index b1b35ed940..063b1d1bd0 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -477,24 +477,24 @@ module ActiveRecord
 
     def create_or_update
       raise ReadOnlyRecord if readonly?
-      result = new_record? ? create_record : update_record
+      result = new_record? ? _create_record : _update_record
       result != false
     end
 
     # Updates the associated record with values matching those of the instance attributes.
     # Returns the number of affected rows.
-    def update_record(attribute_names = @attributes.keys)
+    def _update_record(attribute_names = @attributes.keys)
       attributes_values = arel_attributes_with_values_for_update(attribute_names)
       if attributes_values.empty?
         0
       else
-        self.class.unscoped.update_record attributes_values, id, id_was
+        self.class.unscoped._update_record attributes_values, id, id_was
       end
     end
 
     # Creates a record with values matching those of the instance attributes
     # and returns its id.
-    def create_record(attribute_names = @attributes.keys)
+    def _create_record(attribute_names = @attributes.keys)
       attributes_values = arel_attributes_with_values_for_create(attribute_names)
 
       new_id = self.class.unscoped.insert attributes_values
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index fb213dc6f7..447042254d 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -70,7 +70,7 @@ module ActiveRecord
         binds)
     end
 
-    def update_record(values, id, id_was) # :nodoc:
+    def _update_record(values, id, id_was) # :nodoc:
       substitutes, binds = substitute_values values
       um = @klass.unscoped.where(@klass.arel_table[@klass.primary_key].eq(id_was || id)).arel.compile_update(substitutes, @klass.primary_key)
 
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index 7178bed560..6c30ccab72 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -43,7 +43,7 @@ module ActiveRecord
 
   private
 
-    def create_record
+    def _create_record
       if self.record_timestamps
         current_time = current_time_from_proper_timezone
 
@@ -57,7 +57,7 @@ module ActiveRecord
       super
     end
 
-    def update_record(*args)
+    def _update_record(*args)
       if should_record_timestamps?
         current_time = current_time_from_proper_timezone
 
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 <jeffersonlai@Jeffersons-MacBook-Pro.local>
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/CHANGELOG.md                                       | 9 +++++++++
 activerecord/lib/active_record/associations/collection_proxy.rb | 4 ++++
 activerecord/test/cases/relations_test.rb                       | 6 ++++++
 activerecord/test/models/post.rb                                | 4 ++++
 4 files changed, 23 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1aeba4f856..d31551d606 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,12 @@
+*   `to_sql` on an association now matches the query that is actually executed, where it
+    could previously have incorrectly accrued additional conditions (e.g. as a result of
+    a previous query). CollectionProxy now always defers to the association scope's
+    `arel` method so the (incorrect) inherited one should be entirely concealed.
+
+    Fixes #14003.
+
+    *Jefferson Lai*
+
 *   The PostgreSQL adapter supports custom domains. Fixes #14305.
 
     *Yves Senn*
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index eba688866c..5b71ed163e 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -860,6 +860,10 @@ module ActiveRecord
         !!@association.include?(record)
       end
 
+      def arel
+        scope.arel
+      end
+
       def proxy_association
         @association
       end
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 }
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index faf539a562..099e039255 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -149,6 +149,10 @@ class Post < ActiveRecord::Base
     ranked_by_comments.limit_by(limit)
   end
 
+  def self.written_by(author)
+    where(id: author.posts.pluck(:id))
+  end
+
   def self.reset_log
     @log = []
   end
-- 
cgit v1.2.3


From f4226c3ab6651f6871e02f3c6754c29ab155b938 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
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/CHANGELOG.md                                           | 6 ++++++
 .../lib/active_record/connection_adapters/postgresql_adapter.rb     | 2 +-
 .../lib/active_record/connection_adapters/sqlite3_adapter.rb        | 2 +-
 activerecord/test/cases/adapters/postgresql/array_test.rb           | 2 +-
 activerecord/test/cases/reflection_test.rb                          | 2 +-
 activerecord/test/schema/schema.rb                                  | 2 +-
 6 files changed, 11 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 98fe0fbd62..dce2692b0d 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   PostgreSQL and SQLite string columns no longer have a default limit of 255.
+
+    Fixes #13435, #9153.
+
+    *Vladimir Sazhin*, *Toms Mikoss*, *Yves Senn*
+
 *   Block a few default Class methods as scope name.
 
     For instance, this will raise:
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 9fe8e0497e..3510e4f3b0 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -209,7 +209,7 @@ module ActiveRecord
 
       NATIVE_DATABASE_TYPES = {
         primary_key: "serial primary key",
-        string:      { name: "character varying", limit: 255 },
+        string:      { name: "character varying" },
         text:        { name: "text" },
         integer:     { name: "integer" },
         float:       { name: "float" },
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 6e6a51dab8..cd1f7a16c6 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -63,7 +63,7 @@ module ActiveRecord
 
       NATIVE_DATABASE_TYPES = {
         primary_key:  'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL',
-        string:       { name: "varchar", limit: 255 },
+        string:       { name: "varchar" },
         text:         { name: "text" },
         integer:      { name: "integer" },
         float:        { name: "float" },
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
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index a9c4980283..d79cce8cca 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -673,7 +673,7 @@ ActiveRecord::Schema.define do
   end
 
   create_table :topics, force: true do |t|
-    t.string   :title
+    t.string   :title, limit: 250
     t.string   :author_name
     t.string   :author_email_address
     if mysql_56?
-- 
cgit v1.2.3


From 80f4a65bbd2372c01cfdf174e6446cd232d81e44 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
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')

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 989cccdc3e6f9c6dcb740b2792fcc74b19eed67e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
Date: Fri, 4 Apr 2014 18:41:43 -0300
Subject: Add CHANGELOG entry for #11650 [ci skip]

---
 activerecord/CHANGELOG.md | 6 ++++++
 1 file changed, 6 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index fff66f21fb..4d555bd33e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Make possible to have an association called `records`.
+
+    Fixes #11645.
+
+    *prathamesh-sonpatki*
+
 *   `to_sql` on an association now matches the query that is actually executed, where it
     could previously have incorrectly accrued additional conditions (e.g. as a result of
     a previous query). CollectionProxy now always defers to the association scope's
-- 
cgit v1.2.3


From 57c7d5cb80086e66eee3303b11a5e558aa9e5bf2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
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 +++++---------------
 activerecord/test/models/column.rb                   |  3 +++
 activerecord/test/models/record.rb                   |  2 ++
 activerecord/test/schema/schema.rb                   |  6 ++++++
 4 files changed, 16 insertions(+), 15 deletions(-)
 create mode 100644 activerecord/test/models/column.rb
 create mode 100644 activerecord/test/models/record.rb

(limited to 'activerecord')

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
diff --git a/activerecord/test/models/column.rb b/activerecord/test/models/column.rb
new file mode 100644
index 0000000000..499358b4cf
--- /dev/null
+++ b/activerecord/test/models/column.rb
@@ -0,0 +1,3 @@
+class Column < ActiveRecord::Base
+  belongs_to :record
+end
diff --git a/activerecord/test/models/record.rb b/activerecord/test/models/record.rb
new file mode 100644
index 0000000000..f77ac9fc03
--- /dev/null
+++ b/activerecord/test/models/record.rb
@@ -0,0 +1,2 @@
+class Record < ActiveRecord::Base
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index a9c4980283..5bd3a51a84 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -161,6 +161,10 @@ ActiveRecord::Schema.define do
     t.integer :references, null: false
   end
 
+  create_table :columns, force: true do |t|
+    t.references :record
+  end
+
   create_table :comments, force: true do |t|
     t.integer :post_id, null: false
     # use VARCHAR2(4000) instead of CLOB datatype as CLOB data type has many limitations in
@@ -819,6 +823,8 @@ ActiveRecord::Schema.define do
     t.integer :department_id
   end
 
+  create_table :records, force: true do |t|
+  end
 
   except 'SQLite' do
     # fk_test_has_fk should be before fk_test_has_pk
-- 
cgit v1.2.3


From 9534006c47c9aa564a55de714d1e0c8699fd185a Mon Sep 17 00:00:00 2001
From: eileencodes <eileencodes@gmail.com>
Date: Thu, 3 Apr 2014 18:20:28 -0400
Subject: fix CollectionProxy delete_all documentation

This method no longer returns an array of all records that
have been removed. Correct documentation to reflect this change.

See issue 14546
---
 activerecord/lib/active_record/associations/collection_proxy.rb | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 5b71ed163e..84c8cfe72b 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -357,7 +357,7 @@ module ActiveRecord
 
       # Deletes all the records from the collection. For +has_many+ associations,
       # the deletion is done according to the strategy specified by the <tt>:dependent</tt>
-      # option. Returns an array with the deleted records.
+      # option.
       #
       # If no <tt>:dependent</tt> option is given, then it will follow the
       # default strategy. The default strategy is <tt>:nullify</tt>. This
@@ -435,11 +435,6 @@ module ActiveRecord
       #   #    ]
       #
       #   person.pets.delete_all
-      #   # => [
-      #   #       #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>,
-      #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
-      #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
-      #   #    ]
       #
       #   Pet.find(1, 2, 3)
       #   # => ActiveRecord::RecordNotFound
-- 
cgit v1.2.3


From bbe7fe41692e5a2c869f812616d0c99a2bfcb39c Mon Sep 17 00:00:00 2001
From: Evan Whalen <evanwhalen@gmail.com>
Date: Mon, 7 Apr 2014 10:01:03 -0400
Subject: make enums distinct per class

---
 activerecord/CHANGELOG.md              |  7 +++++++
 activerecord/lib/active_record/enum.rb | 10 ++++++----
 activerecord/test/cases/enum_test.rb   | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 16f9db95fc..d39e808f5b 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Fixed a problem where an enum would overwrite values of another enum
+    with the same name in an unrelated class.
+
+    Fixes #14607.
+
+    *Evan Whalen*
+
 *   PostgreSQL and SQLite string columns no longer have a default limit of 255.
 
     Fixes #13435, #9153.
diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 4aa323fb00..167f8b7a49 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -65,10 +65,13 @@ module ActiveRecord
   #
   # Where conditions on an enum attribute must use the ordinal value of an enum.
   module Enum
-    DEFINED_ENUMS = {} # :nodoc:
+    def self.extended(base)
+      base.class_attribute(:defined_enums)
+      base.defined_enums = {}
+    end
 
     def enum_mapping_for(attr_name) # :nodoc:
-      DEFINED_ENUMS[attr_name.to_s]
+      defined_enums[attr_name.to_s]
     end
 
     def enum(definitions)
@@ -122,9 +125,8 @@ module ActiveRecord
             klass.send(:detect_enum_conflict!, name, value, true)
             klass.scope value, -> { klass.where name => i }
           end
-
-          DEFINED_ENUMS[name.to_s] = enum_values
         end
+        defined_enums[name.to_s] = enum_values
       end
     end
 
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 <laurocaetano1@gmail.com>
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/lib/active_record/relation/calculations.rb |  2 +-
 activerecord/test/cases/calculations_test.rb            | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 45ffb99868..812e3e800a 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -231,7 +231,7 @@ module ActiveRecord
 
     def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
       # Postgresql doesn't like ORDER BY when there are no GROUP BY
-      relation = reorder(nil)
+      relation = unscope(:order)
 
       column_alias = column_name
 
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 <matthew@trebex.net>
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.
---
 .../connection_adapters/postgresql/quoting.rb           |  4 ++--
 .../test/cases/adapters/postgresql/bytea_test.rb        | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
index 210172cf32..ac3b0f713d 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -4,14 +4,14 @@ module ActiveRecord
       module Quoting
         # Escapes binary strings for bytea input to the database.
         def escape_bytea(value)
-          PGconn.escape_bytea(value) if value
+          @connection.escape_bytea(value) if value
         end
 
         # Unescapes bytea output from a database to the binary string it represents.
         # NOTE: This is NOT an inverse of escape_bytea! This is only to be used
         # on escaped binary output from database drive.
         def unescape_bytea(value)
-          PGconn.unescape_bytea(value) if value
+          @connection.unescape_bytea(value) if value
         end
 
         # Quotes PostgreSQL-specific data types for SQL input.
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 <laurocaetano1@gmail.com>
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/lib/active_record/relation/merger.rb        |  1 -
 activerecord/lib/active_record/relation/query_methods.rb |  6 +++---
 activerecord/test/cases/relation/mutation_test.rb        | 14 +++++++++++---
 activerecord/test/cases/relations_test.rb                | 12 ++++++++++++
 4 files changed, 26 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 182b9ed89c..be44fccad5 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -139,7 +139,6 @@ module ActiveRecord
       def merge_single_values
         relation.from_value          = values[:from] unless relation.from_value
         relation.lock_value          = values[:lock] unless relation.lock_value
-        relation.reverse_order_value = values[:reverse_order]
 
         unless values[:create_with].blank?
           relation.create_with_value = (relation.create_with_value || {}).merge(values[:create_with])
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 0213bca981..4287304945 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -825,7 +825,9 @@ module ActiveRecord
     end
 
     def reverse_order! # :nodoc:
-      self.reverse_order_value = !reverse_order_value
+      orders = order_values.uniq
+      orders.reject!(&:blank?)
+      self.order_values = reverse_sql_order(orders)
       self
     end
 
@@ -871,7 +873,6 @@ module ActiveRecord
 
       case scope
       when :order
-        self.reverse_order_value = false
         result = []
       else
         result = [] unless single_val_method
@@ -1031,7 +1032,6 @@ module ActiveRecord
     def build_order(arel)
       orders = order_values.uniq
       orders.reject!(&:blank?)
-      orders = reverse_sql_order(orders) if reverse_order_value
 
       arel.order(*orders) unless orders.empty?
     end
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 2512bd717bbf60364935afdbacb422d9248a1ba8 Mon Sep 17 00:00:00 2001
From: eileencodes <eileencodes@gmail.com>
Date: Mon, 7 Apr 2014 14:16:55 -0400
Subject: remove check for present? from delete_all

Passing in a blank string is a bug so there's no reason
to check for a blank string.
---
 activerecord/lib/active_record/associations/collection_association.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 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 dee7e972c1..803e3ab9ab 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -182,11 +182,11 @@ module ActiveRecord
       #
       # See delete for more info.
       def delete_all(dependent = nil)
-        if dependent.present? && ![:nullify, :delete_all].include?(dependent)
+        if dependent && ![:nullify, :delete_all].include?(dependent)
           raise ArgumentError, "Valid values are :nullify or :delete_all"
         end
 
-        dependent = if dependent.present?
+        dependent = if dependent
                       dependent
                     elsif options[:dependent] == :destroy
                       :delete_all
-- 
cgit v1.2.3


From a5664fe27b1797983537c0764003e618bd3d2801 Mon Sep 17 00:00:00 2001
From: Godfrey Chan <godfreykfc@gmail.com>
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/lib/active_record/enum.rb             |  9 ++++--
 .../lib/active_record/validations/uniqueness.rb    |  2 +-
 activerecord/test/cases/enum_test.rb               | 35 +++++++++++++++++-----
 3 files changed, 35 insertions(+), 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 167f8b7a49..6e0d19ad31 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -1,3 +1,5 @@
+require 'active_support/core_ext/object/deep_dup'
+
 module ActiveRecord
   # Declare an enum attribute where the values map to integers in the database,
   # but can be queried by name. Example:
@@ -70,8 +72,9 @@ module ActiveRecord
       base.defined_enums = {}
     end
 
-    def enum_mapping_for(attr_name) # :nodoc:
-      defined_enums[attr_name.to_s]
+    def inherited(base)
+      base.defined_enums = defined_enums.deep_dup
+      super
     end
 
     def enum(definitions)
@@ -136,7 +139,7 @@ module ActiveRecord
           mod = Module.new do
             private
               def save_changed_attribute(attr_name, value)
-                if (mapping = self.class.enum_mapping_for(attr_name))
+                if (mapping = self.class.defined_enums[attr_name])
                   if attribute_changed?(attr_name)
                     old = changed_attributes[attr_name]
 
diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb
index 71c71cb4b1..ee080451a9 100644
--- a/activerecord/lib/active_record/validations/uniqueness.rb
+++ b/activerecord/lib/active_record/validations/uniqueness.rb
@@ -93,7 +93,7 @@ module ActiveRecord
       end
 
       def map_enum_attribute(klass, attribute, value)
-        mapping = klass.enum_mapping_for(attribute.to_s)
+        mapping = klass.defined_enums[attribute.to_s]
         value = mapping[value] if value && mapping
         value
       end
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 77692ff9c32c1b4999bd4ff51af6b84186729478 Mon Sep 17 00:00:00 2001
From: Godfrey Chan <godfreykfc@gmail.com>
Date: Mon, 7 Apr 2014 19:08:08 -0700
Subject: Ensure we are looking  with string keys

---
 activerecord/lib/active_record/enum.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 6e0d19ad31..18f1ca26de 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -139,7 +139,7 @@ module ActiveRecord
           mod = Module.new do
             private
               def save_changed_attribute(attr_name, value)
-                if (mapping = self.class.defined_enums[attr_name])
+                if (mapping = self.class.defined_enums[attr_name.to_s])
                   if attribute_changed?(attr_name)
                     old = changed_attributes[attr_name]
 
-- 
cgit v1.2.3


From 2b817cde622e625f08a638d909f3e9b12f0b3066 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
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.
---
 .../lib/active_record/connection_handling.rb       | 42 +++++++--------------
 .../connection_adapters/connection_handler_test.rb | 43 +++++++++++++---------
 2 files changed, 40 insertions(+), 45 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index bbb866cedf..5a83797ee5 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -58,9 +58,8 @@ module ActiveRecord
     end
 
     class MergeAndResolveDefaultUrlConfig # :nodoc:
-      def initialize(raw_configurations, url = ENV['DATABASE_URL'])
+      def initialize(raw_configurations)
         @raw_config = raw_configurations.dup
-        @url        = url
       end
 
       # Returns fully resolved connection hashes.
@@ -71,35 +70,22 @@ module ActiveRecord
 
       private
         def config
-          if @url
-            raw_merged_into_default
-          else
-            @raw_config
+          cfg = Hash.new do |hash, key|
+            entry = @raw_config[key]
+            env_url = nil
+            if key.to_s == DEFAULT_ENV.call.to_s
+              env_url = ENV["DATABASE_URL"]
+            end
+            env_url ||= ENV["DATABASE_URL_#{key.upcase}"]
+            entry ||= {} if env_url
+            entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash)
+            hash[key] = entry if entry
           end
-        end
 
-        def raw_merged_into_default
-          default = default_url_hash
+          @raw_config.keys.each {|k| cfg[k] }
+          cfg[DEFAULT_ENV.call.to_s]
 
-          @raw_config.each do |env, values|
-            default[env] = values || {}
-            default[env].merge!("url" => @url) { |h, v1, v2| v1 || v2 } if default[env].is_a?(Hash)
-          end
-          default
-        end
-
-        # When the raw configuration is not present and ENV['DATABASE_URL']
-        # is available we return a hash with the connection information in
-        # the connection URL. This hash responds to any string key with
-        # resolved connection information.
-        def default_url_hash
-          Hash.new do |hash, key|
-            hash[key] = if key.is_a? String
-               ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(@url).to_hash
-            else
-              nil
-            end
-          end
+          cfg
         end
     end
 
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?=
 <rafaelmfranca@gmail.com>
Date: Mon, 7 Apr 2014 19:05:50 -0300
Subject: Make sure DEFAULT_URL only override current environment database
 configuration

---
 .../lib/active_record/connection_handling.rb       |  2 +-
 .../connection_adapters/connection_handler_test.rb | 38 +++++++++++++++++-----
 2 files changed, 30 insertions(+), 10 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 5a83797ee5..567d121091 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -78,7 +78,7 @@ module ActiveRecord
             end
             env_url ||= ENV["DATABASE_URL_#{key.upcase}"]
             entry ||= {} if env_url
-            entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash)
+            entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash) && env_url
             hash[key] = entry if entry
           end
 
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?=
 <rafaelmfranca@gmail.com>
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')

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 2b6d1da6b4f2a5dc1d51223ddb33b86ceab98479 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
Date: Mon, 7 Apr 2014 19:17:06 -0300
Subject: Only call DEFAULT_ENV proc one time

---
 activerecord/lib/active_record/connection_handling.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 567d121091..a1fb83fde5 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -70,10 +70,13 @@ module ActiveRecord
 
       private
         def config
+          env = DEFAULT_ENV.call.to_s
+
           cfg = Hash.new do |hash, key|
             entry = @raw_config[key]
             env_url = nil
-            if key.to_s == DEFAULT_ENV.call.to_s
+
+            if key.to_s == env
               env_url = ENV["DATABASE_URL"]
             end
             env_url ||= ENV["DATABASE_URL_#{key.upcase}"]
@@ -83,7 +86,7 @@ module ActiveRecord
           end
 
           @raw_config.keys.each {|k| cfg[k] }
-          cfg[DEFAULT_ENV.call.to_s]
+          cfg[env]
 
           cfg
         end
-- 
cgit v1.2.3


From d035ba143a46e64e6110b6a690e0655e8574fd49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
Date: Mon, 7 Apr 2014 19:17:43 -0300
Subject: Check env_url only once

---
 activerecord/lib/active_record/connection_handling.rb | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index a1fb83fde5..08849b3722 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -79,9 +79,14 @@ module ActiveRecord
             if key.to_s == env
               env_url = ENV["DATABASE_URL"]
             end
+
             env_url ||= ENV["DATABASE_URL_#{key.upcase}"]
-            entry ||= {} if env_url
-            entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash) && env_url
+
+            if env_url
+              entry ||= {}
+              entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash)
+            end
+
             hash[key] = entry if entry
           end
 
-- 
cgit v1.2.3


From 0ec047533a1d1ed14e001cfe7fdee818c0c18124 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
Date: Mon, 7 Apr 2014 19:19:28 -0300
Subject: entry is always a Hash

---
 activerecord/lib/active_record/connection_handling.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 08849b3722..1c8fbcb625 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -84,7 +84,7 @@ module ActiveRecord
 
             if env_url
               entry ||= {}
-              entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 } if entry.is_a?(Hash)
+              entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 }
             end
 
             hash[key] = entry if entry
-- 
cgit v1.2.3


From de9f2f63b8548e2a8950c1727f0a1a6893713505 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Tue, 8 Apr 2014 14:49:03 +0930
Subject: Rearrange the config merger some more

This seems to simplify the operative part. Most importantly, by
pre-loading all the configs supplied in ENV, we ensure the list is
complete: if the developer specifies an unknown config, the exception
includes a list of valid ones.
---
 .../lib/active_record/connection_handling.rb       | 35 ++++++++++------------
 1 file changed, 16 insertions(+), 19 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 1c8fbcb625..f83829bb26 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -60,6 +60,7 @@ module ActiveRecord
     class MergeAndResolveDefaultUrlConfig # :nodoc:
       def initialize(raw_configurations)
         @raw_config = raw_configurations.dup
+        @env = DEFAULT_ENV.call.to_s
       end
 
       # Returns fully resolved connection hashes.
@@ -70,30 +71,26 @@ module ActiveRecord
 
       private
         def config
-          env = DEFAULT_ENV.call.to_s
-
-          cfg = Hash.new do |hash, key|
-            entry = @raw_config[key]
-            env_url = nil
-
-            if key.to_s == env
-              env_url = ENV["DATABASE_URL"]
+          @raw_config.dup.tap do |cfg|
+            urls_in_environment.each do |key, url|
+              cfg[key] ||= {}
+              cfg[key]["url"] ||= url
             end
+          end
+        end
 
-            env_url ||= ENV["DATABASE_URL_#{key.upcase}"]
-
-            if env_url
-              entry ||= {}
-              entry.merge!("url" => env_url) { |h, v1, v2| v1 || v2 }
+        def urls_in_environment
+          {}.tap do |mapping|
+            ENV.each do |k, v|
+              if k =~ /\ADATABASE_URL_(.*)/
+                mapping[$1.downcase] = v
+              end
             end
 
-            hash[key] = entry if entry
+            # Check for this last, because it is prioritised over the
+            # longer "DATABASE_URL_#{@env}" spelling
+            mapping[@env] = ENV['DATABASE_URL'] if ENV['DATABASE_URL']
           end
-
-          @raw_config.keys.each {|k| cfg[k] }
-          cfg[env]
-
-          cfg
         end
     end
 
-- 
cgit v1.2.3


From 23692184bbcc2e411ed6bc6d1eaea09aa0f0474f Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
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.
---
 .../active_record/connection_adapters/connection_specification.rb   | 4 ++--
 .../test/cases/connection_adapters/connection_handler_test.rb       | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
index e0715f7ce9..0cc92fa397 100644
--- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
@@ -220,10 +220,10 @@ module ActiveRecord
           # an environment key or a URL spec, so we have deprecated
           # this ambiguous behaviour and in the future this function
           # can be removed in favor of resolve_url_connection.
-          if configurations.key?(spec)
+          if configurations.key?(spec) || spec !~ /:/
             ActiveSupport::Deprecation.warn "Passing a string to ActiveRecord::Base.establish_connection " \
               "for a configuration lookup is deprecated, please pass a symbol (#{spec.to_sym.inspect}) instead"
-            resolve_connection(configurations[spec])
+            resolve_symbol_connection(spec)
           else
             resolve_url_connection(spec)
           end
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 <matthew@trebex.net>
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')

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 <matthew@trebex.net>
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.
---
 .../lib/active_record/connection_handling.rb       | 32 +++++-----
 .../connection_adapters/connection_handler_test.rb | 70 ++++------------------
 2 files changed, 27 insertions(+), 75 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index f83829bb26..3667a42864 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -72,24 +72,24 @@ module ActiveRecord
       private
         def config
           @raw_config.dup.tap do |cfg|
-            urls_in_environment.each do |key, url|
-              cfg[key] ||= {}
-              cfg[key]["url"] ||= url
-            end
-          end
-        end
-
-        def urls_in_environment
-          {}.tap do |mapping|
-            ENV.each do |k, v|
-              if k =~ /\ADATABASE_URL_(.*)/
-                mapping[$1.downcase] = v
+            if url = ENV['DATABASE_URL']
+              if cfg[@env]
+                if cfg[@env]["url"]
+                  # Nothing to do
+                else
+                  ActiveSupport::Deprecation.warn "Overriding database configuration with DATABASE_URL without using an ERB tag in database.yml is deprecated. Please update the entry for #{@env.inspect}:\n\n" \
+                    "  #{@env}:\n    url: <%= ENV['DATABASE_URL'] %>\n\n"\
+                    "This will be required in Rails 4.2"
+                  cfg[@env]["url"] = url
+                end
+              else
+                cfg[@env] = {}
+                ActiveSupport::Deprecation.warn "Supplying DATABASE_URL without a matching entry in database.yml is deprecated. Please add an entry for #{@env.inspect}:\n\n" \
+                  "  #{@env}:\n    url: <%= ENV['DATABASE_URL'] %>\n\n"\
+                  "This will be required in Rails 4.2"
+                cfg[@env]["url"] ||= url
               end
             end
-
-            # Check for this last, because it is prioritised over the
-            # longer "DATABASE_URL_#{@env}" spelling
-            mapping[@env] = ENV['DATABASE_URL'] if ENV['DATABASE_URL']
           end
         end
     end
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 <matthew@trebex.net>
Date: Wed, 9 Apr 2014 00:55:49 +0930
Subject: Don't deprecate after all

---
 .../lib/active_record/connection_handling.rb         | 18 ++----------------
 .../connection_adapters/connection_handler_test.rb   | 20 +++++++++-----------
 2 files changed, 11 insertions(+), 27 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 3667a42864..31e7390bf7 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -73,22 +73,8 @@ module ActiveRecord
         def config
           @raw_config.dup.tap do |cfg|
             if url = ENV['DATABASE_URL']
-              if cfg[@env]
-                if cfg[@env]["url"]
-                  # Nothing to do
-                else
-                  ActiveSupport::Deprecation.warn "Overriding database configuration with DATABASE_URL without using an ERB tag in database.yml is deprecated. Please update the entry for #{@env.inspect}:\n\n" \
-                    "  #{@env}:\n    url: <%= ENV['DATABASE_URL'] %>\n\n"\
-                    "This will be required in Rails 4.2"
-                  cfg[@env]["url"] = url
-                end
-              else
-                cfg[@env] = {}
-                ActiveSupport::Deprecation.warn "Supplying DATABASE_URL without a matching entry in database.yml is deprecated. Please add an entry for #{@env.inspect}:\n\n" \
-                  "  #{@env}:\n    url: <%= ENV['DATABASE_URL'] %>\n\n"\
-                  "This will be required in Rails 4.2"
-                cfg[@env]["url"] ||= url
-              end
+              cfg[@env] ||= {}
+              cfg[@env]["url"] ||= url
             end
           end
         end
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 <matthew@trebex.net>
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.
---
 .../active_record/connection_adapters/connection_specification.rb | 2 +-
 .../test/cases/connection_adapters/connection_handler_test.rb     | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
index 0cc92fa397..a8ab52be74 100644
--- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
@@ -33,7 +33,7 @@ module ActiveRecord
         def initialize(url)
           raise "Database URL cannot be empty" if url.blank?
           @uri     = URI.parse(url)
-          @adapter = @uri.scheme
+          @adapter = @uri.scheme.gsub('-', '_')
           @adapter = "postgresql" if @adapter == "postgres"
 
           if @uri.opaque
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 a91e5ff2cbf2f8971ad0d2a80b2e22e63b1900f6 Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
Date: Wed, 9 Apr 2014 14:39:08 -0300
Subject: The `source` option for `has_many => through` should accept String
 values.

With the changes introduced by 16b70fddd4dc7e7fb7be108add88bae6e3c2509b
it was expecting the value to be a Symbol, while it could be also a
String value.
---
 activerecord/lib/active_record/reflection.rb | 2 +-
 activerecord/test/models/tag.rb              | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 03b5bdc46c..5465a7bfd7 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -621,7 +621,7 @@ module ActiveRecord
       end
 
       def source_reflection_name # :nodoc:
-        return @source_reflection_name if @source_reflection_name
+        return @source_reflection_name.to_sym if @source_reflection_name
 
         names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq
         names = names.find_all { |n|
diff --git a/activerecord/test/models/tag.rb b/activerecord/test/models/tag.rb
index a581b381e8..80d4725f7e 100644
--- a/activerecord/test/models/tag.rb
+++ b/activerecord/test/models/tag.rb
@@ -3,5 +3,5 @@ class Tag < ActiveRecord::Base
   has_many :taggables, :through => :taggings
   has_one  :tagging
 
-  has_many :tagged_posts, :through => :taggings, :source => :taggable, :source_type => 'Post'
-end
\ No newline at end of file
+  has_many :tagged_posts, :through => :taggings, :source => 'taggable', :source_type => 'Post'
+end
-- 
cgit v1.2.3


From 1f31488499111fdfce79d8dc1cc8fb008f7cdb25 Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
Date: Wed, 9 Apr 2014 21:23:43 -0300
Subject: Make the reflections cache work with strings as its keys.

---
 activerecord/lib/active_record/reflection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 5465a7bfd7..9e4c70dabf 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -22,7 +22,7 @@ module ActiveRecord
     end
 
     def self.add_reflection(ar, name, reflection)
-      ar.reflections = ar.reflections.merge(name => reflection)
+      ar.reflections = ar.reflections.merge(name.to_s => reflection)
     end
 
     def self.add_aggregate_reflection(ar, name, reflection)
@@ -72,7 +72,7 @@ module ActiveRecord
       #   Invoice.reflect_on_association(:line_items).macro  # returns :has_many
       #
       def reflect_on_association(association)
-        reflections[association]
+        reflections[association.to_s]
       end
 
       # Returns an array of AssociationReflection objects for all associations which have <tt>:autosave</tt> enabled.
-- 
cgit v1.2.3


From 4c6ba7432dc5288708817d5b03e7ed13458ca58d Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
Date: Wed, 9 Apr 2014 21:25:39 -0300
Subject: No need to call `to_sym` on reflection name, since the cache now
 works with strings with string keys.

Related #14668.
---
 activerecord/lib/active_record/reflection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 9e4c70dabf..eddc0cf51c 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -617,11 +617,11 @@ module ActiveRecord
       #   # => [:tag, :tags]
       #
       def source_reflection_names
-        (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }.uniq
+        (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n }.uniq
       end
 
       def source_reflection_name # :nodoc:
-        return @source_reflection_name.to_sym if @source_reflection_name
+        return @source_reflection_name if @source_reflection_name
 
         names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq
         names = names.find_all { |n|
-- 
cgit v1.2.3


From e7630b62a604a0bc11a56283140f11a80e4cc1b0 Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
Date: Wed, 9 Apr 2014 21:34:48 -0300
Subject: Make the aggregate_reflections cache work with strings as its keys.

---
 activerecord/lib/active_record/reflection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index eddc0cf51c..1ce477f38b 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -26,7 +26,7 @@ module ActiveRecord
     end
 
     def self.add_aggregate_reflection(ar, name, reflection)
-      ar.aggregate_reflections = ar.aggregate_reflections.merge(name => reflection)
+      ar.aggregate_reflections = ar.aggregate_reflections.merge(name.to_s => reflection)
     end
 
     # \Reflection enables to interrogate Active Record classes and objects
@@ -48,7 +48,7 @@ module ActiveRecord
       #   Account.reflect_on_aggregation(:balance) # => the balance AggregateReflection
       #
       def reflect_on_aggregation(aggregation)
-        aggregate_reflections[aggregation]
+        aggregate_reflections[aggregation.to_s]
       end
 
       # Returns an array of AssociationReflection objects for all the
-- 
cgit v1.2.3


From 213ef567ae2ab92f1e1145cd385da0c5b9534422 Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
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')

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


From cf0e44c771319bcd8ec0038f8502fe0baa5fa738 Mon Sep 17 00:00:00 2001
From: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Date: Wed, 9 Apr 2014 23:30:47 -0300
Subject: Remove extra collect call

---
 activerecord/lib/active_record/reflection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 1ce477f38b..22ee2238ce 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -617,7 +617,7 @@ module ActiveRecord
       #   # => [:tag, :tags]
       #
       def source_reflection_names
-        (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n }.uniq
+        (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).uniq
       end
 
       def source_reflection_name # :nodoc:
-- 
cgit v1.2.3


From 9a8af0c15089fbdc18fa75115586ab6febcd4d06 Mon Sep 17 00:00:00 2001
From: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Date: Wed, 9 Apr 2014 23:31:10 -0300
Subject: Only call uniq on the conditional that actually needs it

---
 activerecord/lib/active_record/reflection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 22ee2238ce..1724ea95b0 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -617,7 +617,7 @@ module ActiveRecord
       #   # => [:tag, :tags]
       #
       def source_reflection_names
-        (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).uniq
+        options[:source] ? [options[:source]] : [name.to_s.singularize, name].uniq
       end
 
       def source_reflection_name # :nodoc:
-- 
cgit v1.2.3