From 4d5e6607899832bde1365bdd4f7617a24bca4561 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@thoughtbot.com>
Date: Sat, 10 Jan 2015 11:57:14 -0700
Subject: Don't attempt to save dirty attributes which are not persistable

This sets a precident for how we handle `attribute` calls, which aren't
backed by a database column. We should not take this as a conscious
decision on how to handle them, and this can change when we make
`attribute` public if we have better ideas in the future.

As the composed attributes API gets fleshed out, I expect the
`persistable_attributes` method to change to
`@attributes.select(&:persistable).keys`, or some more performant
variant there-of. This can probably go away completely once we fully
move dirty checking into the attribute objects once it gets moved up to
Active Model.

Fixes #18407
---
 activerecord/CHANGELOG.md                                 |  7 +++++++
 activerecord/lib/active_record/attribute_methods/dirty.rb |  2 +-
 activerecord/lib/active_record/attributes.rb              |  7 +++++++
 activerecord/test/cases/dirty_test.rb                     | 13 +++++++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c677c41727..c5f0e5022a 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   `attribute_will_change!` will no longer cause non-persistable attributes to
+    be sent to the database.
+
+    Fixes #18407.
+
+    *Sean Griffin*
+
 *   Remove support for the `protected_attributes` gem.
 
     *Carlos Antonio da Silva*, *Roberto Miranda*
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index d5702accaf..ce7f575150 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -134,7 +134,7 @@ module ActiveRecord
       # Serialized attributes should always be written in case they've been
       # changed in place.
       def keys_for_partial_write
-        changed
+        changed & persistable_attribute_names
       end
 
       def _field_changed?(attr, old_value)
diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb
index aafb990bc1..b263a89d79 100644
--- a/activerecord/lib/active_record/attributes.rb
+++ b/activerecord/lib/active_record/attributes.rb
@@ -9,6 +9,8 @@ module ActiveRecord
       class_attribute :user_provided_defaults, instance_accessor: false # :internal:
       self.user_provided_columns = {}
       self.user_provided_defaults = {}
+
+      delegate :persistable_attribute_names, to: :class
     end
 
     module ClassMethods # :nodoc:
@@ -96,6 +98,10 @@ module ActiveRecord
         @columns_hash ||= Hash[columns.map { |c| [c.name, c] }]
       end
 
+      def persistable_attribute_names # :nodoc:
+        @persistable_attribute_names ||= connection.schema_cache.columns_hash(table_name).keys
+      end
+
       def reset_column_information # :nodoc:
         super
         clear_caches_calculated_from_columns
@@ -130,6 +136,7 @@ module ActiveRecord
         @columns_hash = nil
         @content_columns = nil
         @default_attributes = nil
+        @persistable_attribute_names = nil
       end
 
       def raw_default_values
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 5b71bd7e67..ae4a8aab2c 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -708,6 +708,19 @@ class DirtyTest < ActiveRecord::TestCase
     assert model.first_name_changed?
   end
 
+  test "attribute_will_change! doesn't try to save non-persistable attributes" do
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = 'people'
+      attribute :non_persisted_attribute, ActiveRecord::Type::String.new
+    end
+
+    record = klass.new(first_name: "Sean")
+    record.non_persisted_attribute_will_change!
+
+    assert record.non_persisted_attribute_changed?
+    assert record.save
+  end
+
   private
     def with_partial_writes(klass, on = true)
       old = klass.partial_writes?
-- 
cgit v1.2.3